Add support for json_contains_path() #1929
Conversation
…] ...) Verified matched MySQL behavior, which differs from documentation.
There was a problem hiding this comment.
Code and unit tests look good. It needs to be added to the function registry so that we resolve the parsed name into a function on the SQL path for enginetests. Hopefully there are no special name resolution considerations and it will not need special handling in the new planbuilder package. Enginetests and any failures should guide through impl gaps. If there are issues for experimental tests let me know and I can give more info.
| // argument is not a valid path expression, or one_or_all is not 'one' or 'all'. To check for a specific value at a | ||
| // path, use JSON_CONTAINS() instead. | ||
| // | ||
| // The return value is 0 if no specified path exists within the document. Otherwise, the return value depends on the |
There was a problem hiding this comment.
Maybe lead with this? Seems like the important part
There was a problem hiding this comment.
The NULL behavior seems like a footnote. Not sure JSON_CONTAINS comment needs to be here, but if keeping I'd also put after main explanation.
There was a problem hiding this comment.
I wasn't going the docs because this is lifted from the link below. I'm happy to re-write it if you think that's better.
Are you suggesting I need to write Dolt enginetests before I ship this? I didn't know that was required. FWIW, I was testing this via |
I didn't realize at first this was a pre-existing but unsupported function, it's already added to the function registry. Enginetests are our main way of testing GMS and Dolt, we should have some enginetests before releasing a new function. We have |
No description provided.