diff --git a/src/sql-parser/src/ast/defs/expr.rs b/src/sql-parser/src/ast/defs/expr.rs index f19048d092636..ba2ff69ef6fdc 100644 --- a/src/sql-parser/src/ast/defs/expr.rs +++ b/src/sql-parser/src/ast/defs/expr.rs @@ -356,12 +356,7 @@ impl AstDisplay for Expr { } else { f.write_str(op); f.write_str(" "); - // A prefix operator binds tighter than `COLLATE` and the - // binary operators but looser than the postfix `::`/`[…]` - // forms, and `- ` lexes as a negative literal, so a - // low-precedence or numeric-leftmost operand must be - // parenthesized to keep the prefix operator's scope. - if prefix_operand_needs_parens(expr1.as_ref()) { + if prefix_operand_needs_parens(op, expr1.as_ref()) { f.write_str("("); f.write_node(&expr1); f.write_str(")"); @@ -867,8 +862,28 @@ fn prints_self_delimiting(expr: &Expr) -> bool { } } -/// Whether the operand of a prefix operator (`-`/`+`/`~`) must be parenthesized -/// to round-trip. A prefix op binds *tighter* than `COLLATE`/`AT TIME ZONE` and +/// Whether the operand of a prefix operator (an `Op` with no second operand) +/// must be parenthesized so the printed expression reparses with the same +/// structure. Shared by the `AstDisplay` impl and `mz-sql-pretty`, which must +/// agree on when parens are required. +/// +/// A bare `-`/`+` binds at `PrefixPlusMinus`, tighter than every infix +/// operator, so any operand with an exposed operator spine needs parens, plus +/// the `- ` fold and postfix hazards that +/// `bare_prefix_operand_needs_parens` handles. An `Other`-level prefix (`~` +/// or a namespaced `OPERATOR(...)`) instead reparses its operand at `Other`, +/// so the operand re-associates exactly when its left spine exposes that level +/// or looser, the same rule as a binary operator's right operand. +pub fn prefix_operand_needs_parens(op: &Op, operand: &Expr) -> bool { + if unary_prec(op) == prec::PREFIX { + bare_prefix_operand_needs_parens(operand) + } else { + left_edge(operand) <= prec::OTHER + } +} + +/// Whether the operand of a bare prefix `-`/`+` must be parenthesized +/// to round-trip. Such a prefix op binds *tighter* than `COLLATE`/`AT TIME ZONE` and /// the binary/comparison operators, but *looser* than the postfix `::`/`[…]` /// forms — and `- ` additionally lexes as a negative literal. So peel /// the tight postfixes (`::`/`[…]`); if the chain bottoms out at a numeric @@ -876,7 +891,7 @@ fn prints_self_delimiting(expr: &Expr) -> bool { /// than a self-delimiting non-`COLLATE` primary (a `COLLATE`, a binary op, …) the /// prefix op would re-associate — both need parens. (`a + b COLLATE c` reparses /// as `a + (b COLLATE c)`; `- x COLLATE c` as `(- x) COLLATE c`.) -fn prefix_operand_needs_parens(operand: &Expr) -> bool { +fn bare_prefix_operand_needs_parens(operand: &Expr) -> bool { let mut e = operand; let mut saw_postfix = false; loop { diff --git a/src/sql-parser/src/parser.rs b/src/sql-parser/src/parser.rs index bf93d6bb1ce97..5a0b69c2d305f 100644 --- a/src/sql-parser/src/parser.rs +++ b/src/sql-parser/src/parser.rs @@ -695,6 +695,20 @@ impl<'a> Parser<'a> { (Token::Keyword(NOT), _) => Ok(Expr::Not { expr: Box::new(self.parse_subexpr(Precedence::PrefixNot)?), }), + (Token::Keyword(OPERATOR), Some(Token::LParen)) => { + self.expect_token(&Token::LParen)?; + let op = self.parse_operator()?; + self.expect_token(&Token::RParen)?; + // Like PostgreSQL, the operand of a prefix `OPERATOR(...)` + // parses at the precedence of "any other operator", not at + // the tighter precedence of bare unary `+` and `-`, so + // `OPERATOR(pg_catalog.-) 1 + 2` means `- (1 + 2)`. + Ok(Expr::Op { + op, + expr1: Box::new(self.parse_subexpr(Precedence::Other)?), + expr2: None, + }) + } (Token::Keyword(ROW), Some(Token::LParen)) => self.parse_row_expr(), (Token::Keyword(TRIM), Some(Token::LParen)) => self.parse_trim_expr(), (Token::Keyword(POSITION), Some(Token::LParen)) => self.parse_position_expr(), @@ -1556,7 +1570,10 @@ impl<'a> Parser<'a> { Some(Token::Keyword(kw)) => namespace.push(kw.into()), Some(Token::Ident(id)) => namespace.push(self.new_identifier(id)?), Some(Token::Op(op)) => break op, + // The lexer emits `*` and `=` as dedicated tokens rather than + // `Token::Op`, but both are valid operator names here. Some(Token::Star) => break "*".to_string(), + Some(Token::Eq) => break "=".to_string(), tok => self.expected(self.peek_prev_pos(), "operator", tok)?, } self.expect_token(&Token::Dot)?; diff --git a/src/sql-parser/tests/testdata/scalar b/src/sql-parser/tests/testdata/scalar index c15501426aa2f..73cb36de081f6 100644 --- a/src/sql-parser/tests/testdata/scalar +++ b/src/sql-parser/tests/testdata/scalar @@ -678,6 +678,86 @@ parse-scalar ---- Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "+" }, expr1: Value(Number("1")), expr2: Some(Value(Number("2"))) } +parse-scalar +1 OPERATOR(=) 2 +---- +Op { op: Op { namespace: Some([]), op: "=" }, expr1: Value(Number("1")), expr2: Some(Value(Number("2"))) } + +parse-scalar roundtrip +1 OPERATOR(=) 2 +---- +1 OPERATOR(=) 2 + +parse-scalar +1 OPERATOR(pg_catalog.=) 2 +---- +Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "=" }, expr1: Value(Number("1")), expr2: Some(Value(Number("2"))) } + +parse-scalar roundtrip +1 OPERATOR(pg_catalog.=) 2 +---- +1 OPERATOR(pg_catalog.=) 2 + +parse-scalar +OPERATOR(pg_catalog.-) 1 +---- +Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "-" }, expr1: Value(Number("1")), expr2: None } + +parse-scalar roundtrip +OPERATOR(pg_catalog.-) 1 +---- +OPERATOR(pg_catalog.-) 1 + +parse-scalar roundtrip +OPERATOR(-) 1 +---- +OPERATOR(-) 1 + +parse-scalar +OPERATOR(pg_catalog.-) 1 + 2 +---- +Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "-" }, expr1: Op { op: Op { namespace: None, op: "+" }, expr1: Value(Number("1")), expr2: Some(Value(Number("2"))) }, expr2: None } + +parse-scalar roundtrip +OPERATOR(pg_catalog.-) (1 + 2) +---- +OPERATOR(pg_catalog.-) (1 + 2) + +parse-scalar roundtrip +OPERATOR(pg_catalog.-) 1 + 2 +---- +OPERATOR(pg_catalog.-) 1 + 2 + +parse-scalar roundtrip +2 * OPERATOR(pg_catalog.-) 3 + 4 +---- +2 * OPERATOR(pg_catalog.-) 3 + 4 + +parse-scalar roundtrip +~ 1 + 2 +---- +~ 1 + 2 + +parse-scalar +OPERATOR(pg_catalog.-) 1 < 2 +---- +Op { op: Op { namespace: None, op: "<" }, expr1: Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "-" }, expr1: Value(Number("1")), expr2: None }, expr2: Some(Value(Number("2"))) } + +parse-scalar roundtrip +OPERATOR(pg_catalog.-) 1 < 2 +---- +OPERATOR(pg_catalog.-) 1 < 2 + +parse-scalar +2 * OPERATOR(pg_catalog.-) 3 +---- +Op { op: Op { namespace: None, op: "*" }, expr1: Value(Number("2")), expr2: Some(Op { op: Op { namespace: Some([Ident("pg_catalog")]), op: "-" }, expr1: Value(Number("3")), expr2: None }) } + +parse-scalar +OPERATOR(-) OPERATOR(-) 1 +---- +Op { op: Op { namespace: Some([]), op: "-" }, expr1: Op { op: Op { namespace: Some([]), op: "-" }, expr1: Value(Number("1")), expr2: None }, expr2: None } + parse-scalar 1 < ANY (SELECT 2) ---- diff --git a/src/sql-pretty/src/doc.rs b/src/sql-pretty/src/doc.rs index 02c69af4ef4d9..18cbf54d49c63 100644 --- a/src/sql-pretty/src/doc.rs +++ b/src/sql-pretty/src/doc.rs @@ -1217,49 +1217,7 @@ impl Pretty { self.doc_expr(expr2).nest(TAB), ]) } else { - // See the AstDisplay `Expr::Op` comment (`prefix_operand_needs_parens`): - // a prefix op binds tighter than `COLLATE`/the binary ops but - // looser than the postfix `::`/`[…]`, and `- ` folds, so - // peel the tight postfixes and parenthesize when the chain - // bottoms out at a numeric literal or a non-self-delimiting / - // `COLLATE` operand. - let needs_parens = { - let mut e = expr1.as_ref(); - let mut saw_postfix = false; - loop { - match e { - Expr::Cast { expr, .. } | Expr::Subscript { expr, .. } => { - saw_postfix = true; - e = expr.as_ref(); - } - Expr::Value(Value::Number(_)) => break saw_postfix, - // Another prefix operator stacks directly (no - // re-association, no `- ` fold) — safe, - // and avoids exploding deep unary chains. - Expr::Op { expr2: None, .. } | Expr::Not { .. } => break false, - Expr::Value(_) - | Expr::Identifier(_) - | Expr::QualifiedWildcard(_) - | Expr::Parameter(_) - | Expr::Function(_) - | Expr::HomogenizingFunction { .. } - | Expr::NullIf { .. } - | Expr::Subquery(_) - | Expr::Exists(_) - | Expr::Nested(_) - | Expr::Array(_) - | Expr::ArraySubquery(_) - | Expr::List(_) - | Expr::ListSubquery(_) - | Expr::Map(_) - | Expr::MapSubquery(_) - | Expr::Case { .. } - | Expr::Row { .. } => break false, - _ => break true, - } - } - }; - let operand = if needs_parens { + let operand = if prefix_operand_needs_parens(op, expr1.as_ref()) { bracket("(", self.doc_expr(expr1), ")") } else { self.doc_expr(expr1) diff --git a/test/sqllogictest/operator.slt b/test/sqllogictest/operator.slt index 2562ec29dc95b..90df81b5cc0dc 100644 --- a/test/sqllogictest/operator.slt +++ b/test/sqllogictest/operator.slt @@ -54,6 +54,73 @@ select 2 OPERATOR(pg_catalog./) 2 ---- 1 +query I +select 1 where 1 OPERATOR(=) 1; +---- +1 + +query I +select 1 where 1 OPERATOR(pg_catalog.=) 1; +---- +1 + +query I +select 1 where 2 OPERATOR(pg_catalog.<>) 3; +---- +1 + +query I +select 1 where 1 OPERATOR(pg_catalog.=) ANY (ARRAY[1, 2]); +---- +1 + +# Prefix (unary) OPERATOR(). + +query I +select OPERATOR(pg_catalog.-) 1; +---- +-1 + +query I +select OPERATOR(-) 1; +---- +-1 + +query I +select OPERATOR(pg_catalog.+) 5; +---- +5 + +query I +select OPERATOR(pg_catalog.~) 1; +---- +-2 + +# The operand of a prefix OPERATOR() parses at the precedence of "any other +# operator", like in PostgreSQL, so it absorbs binary + and *. +query I +select OPERATOR(pg_catalog.-) 1 + 2; +---- +-3 + +query I +select 2 * OPERATOR(pg_catalog.-) 3 + 4; +---- +-14 + +query I +select 1 where OPERATOR(pg_catalog.-) 1 < 2; +---- +1 + +query I +select OPERATOR(-) OPERATOR(-) 1; +---- +1 + +query error operator does not exist: = integer +select OPERATOR(pg_catalog.=) 1; + query error operator does not exist: mz_catalog.* select 2 OPERATOR(mz_catalog.*) 2; @@ -153,6 +220,24 @@ SHOW CREATE VIEW PG_MULTIPLIER materialize.public.pg_multiplier CREATE VIEW materialize.public.pg_multiplier AS SELECT 2 OPERATOR(pg_catalog.*) 2; +statement ok +CREATE VIEW PG_EQUALER AS SELECT 1 WHERE 1 OPERATOR(pg_catalog.=) 1; + +query TT +SHOW CREATE VIEW PG_EQUALER +---- +materialize.public.pg_equaler +CREATE VIEW materialize.public.pg_equaler AS SELECT 1 WHERE 1 OPERATOR(pg_catalog.=) 1; + +statement ok +CREATE VIEW PG_NEGATER AS SELECT OPERATOR(pg_catalog.-) 1 + 2; + +query TT +SHOW CREATE VIEW PG_NEGATER +---- +materialize.public.pg_negater +CREATE VIEW materialize.public.pg_negater AS SELECT OPERATOR(pg_catalog.-) 1 + 2; + query error Expected operator, found number "5." CREATE VIEW INVALID_FIVE AS select 2 OPERATOR(5.*) 2;