sql-parser: support OPERATOR(pg_catalog.=) and prefix OPERATOR(...)#37643
Draft
ggevay wants to merge 2 commits into
Draft
sql-parser: support OPERATOR(pg_catalog.=) and prefix OPERATOR(...)#37643ggevay wants to merge 2 commits into
ggevay wants to merge 2 commits into
Conversation
The lexer emits = as a dedicated Token::Eq rather than a general Token::Op, and parse_operator only accepted Token::Op and Token::Star. As a result OPERATOR(pg_catalog.=) failed to parse with "Expected operator, found equals sign" while all other operators worked. psqlODBC generates this construct, so the gap breaks real clients. Handle Token::Eq like Token::Star and map it back to the operator name, matching PostgreSQL, which accepts = (and *) inside OPERATOR(). Fixes SQL-486. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PostgreSQL allows OPERATOR(...) in prefix position, for example OPERATOR(pg_catalog.-) 1, but Materialize only parsed the infix form. Add a prefix arm that parses the operand at the precedence of "any other operator", matching PostgreSQL, where a prefix OPERATOR() binds looser than bare unary + and -, so OPERATOR(pg_catalog.-) 1 + 2 means - (1 + 2). Resolution of the named operator reuses the infix path, so non-unary operators fail with "operator does not exist", like PostgreSQL. Printing a prefix operator previously parenthesized any operand with an exposed operator spine. That is wrong for Other-level prefixes (~ and namespaced OPERATOR(...)), whose operand only re-associates when its left spine exposes the Other level or looser. Encode that rule in a prefix_operand_needs_parens helper shared with mz-sql-pretty, which had duplicated the old logic and must agree with AstDisplay for pretty-printed statements to reparse faithfully. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ggevay
force-pushed
the
gabor/sql-486-operator-function-doesnt-work
branch
2 times, most recently
from
05def4b to
aa8f003
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SQL-486.
Motivation
expr OPERATOR(pg_catalog.=) exprfailed to parse withExpected operator, found equals sign, while every other operator worked insideOPERATOR(...). psqlODBC generates this construct routinely, so the gap breaks real ODBC clients. PostgreSQL additionally accepts the prefix form, e.g.SELECT OPERATOR(pg_catalog.-) 1, which Materialize rejected entirely.Description
Two commits:
Accept
=insideOPERATOR(...). The lexer emits=(and*) as dedicated tokens rather than general operator tokens, and theOPERATOR(...)name parser only handled the general token and*. Map=back to its operator name, mirroring the existing*special case.Support the prefix (unary)
OPERATOR(...)form. Matching PostgreSQL, the operand parses at the precedence of "any other operator", looser than bare unary+/-, soOPERATOR(pg_catalog.-) 1 + 2means- (1 + 2). Precedence behavior was verified against PostgreSQL 18, including compositions like2 * OPERATOR(pg_catalog.-) 3 + 4. Operator resolution reuses the infix path, so non-unary operators fail withoperator does not exist, like PostgreSQL. TheOPERATOR(construct is only claimed in expression position when followed by(, sooperatorremains usable as an identifier.The second commit also makes the AST printer's parenthesization of prefix operands precedence-aware. The printer used to over-parenthesize operands of
Other-level prefix operators (~and namespacedOPERATOR(...)), which broke print/reparse fidelity for the new syntax. The rule now lives in one helper shared byAstDisplayandmz-sql-pretty, which previously duplicated the logic.User-facing behavior change:
OPERATOR(pg_catalog.=)(and unqualifiedOPERATOR(=)) now parses, and the prefix operator call syntaxOPERATOR(pg_catalog.-) expris now supported, improving PostgreSQL compatibility, in particular for psqlODBC.Verification
New parser datadriven tests in
src/sql-parser/tests/testdata/scalar(ASTs, round-trips, and precedence pins) and new sqllogictest coverage intest/sqllogictest/operator.slt(results, precedence, error cases, andSHOW CREATE VIEWround-trips through the catalog). The existingmz-sql-prettyfidelity test now exercises the shared parenthesization rule across all parser testdata. Edge-case behavior (precedence, invalid prefix operators,operatoras an identifier) was compared against a live PostgreSQL 18.🤖 Generated with Claude Code