Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/sql-parser/src/ast/defs/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,7 @@ impl<T: AstInfo> AstDisplay for Expr<T> {
} 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 `- <number>` 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(")");
Expand Down Expand Up @@ -867,16 +862,36 @@ fn prints_self_delimiting<T: AstInfo>(expr: &Expr<T>) -> 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 `- <number>` 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<T: AstInfo>(op: &Op, operand: &Expr<T>) -> 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 `- <number>` additionally lexes as a negative literal. So peel
/// the tight postfixes (`::`/`[…]`); if the chain bottoms out at a numeric
/// literal the sign would fold into it, and if it bottoms out at anything other
/// 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<T: AstInfo>(operand: &Expr<T>) -> bool {
fn bare_prefix_operand_needs_parens<T: AstInfo>(operand: &Expr<T>) -> bool {
let mut e = operand;
let mut saw_postfix = false;
loop {
Expand Down
17 changes: 17 additions & 0 deletions src/sql-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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)?;
Expand Down
80 changes: 80 additions & 0 deletions src/sql-parser/tests/testdata/scalar
Original file line number Diff line number Diff line change
Expand Up @@ -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)
----
Expand Down
44 changes: 1 addition & 43 deletions src/sql-pretty/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `- <number>` 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 `- <number>` 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)
Expand Down
85 changes: 85 additions & 0 deletions test/sqllogictest/operator.slt
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
Loading