Support ?-based jsonb operators in Postgres #1242
Conversation
Pull Request Test Coverage Report for Build 8901370315Details
💛 - Coveralls |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @ReppCodes - this looks great. I think we need a few more tests but otherwise 👨🍳 👌
Note @jmhain made some significant changes to how json access is represented in #1215
I think we should merge that PR first and then this PR
| select.selection.unwrap(), | ||
| ); | ||
|
|
||
| let sql = r#"SELECT info FROM orders WHERE info ? 'b'"#; |
There was a problem hiding this comment.
Can we please add coverage for all new operators that were added?
Perhaps just verifying that they roundtrip ( no need to reverify the parsed structure for all of them), perhaps using one_statement_parses_to or something similar
There was a problem hiding this comment.
@alamb The tests at lines 2373 and at 2390 should cover the other of the three new operators added, I believe? Sorry, not sure what I'm missing here.
There was a problem hiding this comment.
Also, I'm cleaning up the conflicts after getting that refactor merged in. Should be up shortly.
There was a problem hiding this comment.
And done. Assuming the three added tests hit the operators as needed, should be good to go. Just let me know!
Thanks!
There was a problem hiding this comment.
Yup --- looks good to me. Thank you (I think it was unclear to me that only three new operators got added initially). Sorry for my confusion
We already support most postgres jsonb operators, but were missing the ?-based ones. This commit adds support for them in both the tokenizer and the AST enums. This is simplified in the tokenizer with a dialect-specific carve-out, since Postgres thankfully does not also use ? for anonymous prepared statement parameters.
ReppCodes
force-pushed
the
postgres_json_ops
branch
from
0b2dc76 to
42f68dc
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @ReppCodes -- looks like a very nice contribution to me 🙏
| select.selection.unwrap(), | ||
| ); | ||
|
|
||
| let sql = r#"SELECT info FROM orders WHERE info ? 'b'"#; |
There was a problem hiding this comment.
Yup --- looks good to me. Thank you (I think it was unclear to me that only three new operators got added initially). Sorry for my confusion
Co-authored-by: Andrew Repp <arepp@cloudflare.com>
3 participants
We already support most postgres jsonb operators, but were missing the ?-based ones. This commit adds support for them in both the tokenizer and the AST enums. This is simplified in the tokenizer with a dialect-specific carve-out, since Postgres thankfully does not also use ? for anonymous prepared statement parameters.
This resolves #1236