LucaCappelletti94 commented on code in PR #2829:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2829#discussion_r4493772397
##########
src/parser/mod.rs:
##########
@@ -2046,13 +2046,15 @@ impl<'a> Parser<'a> {
Ok(Expr::UnaryOp {
op,
expr: Box::new(
-
self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?,
+
self.parse_subexpr(self.dialect.prec_value(Precedence::UnaryPlusMinus))?,
),
})
}
Token::Tilde => Ok(Expr::UnaryOp {
op: UnaryOperator::BitwiseNot,
- expr:
Box::new(self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?),
+ expr: Box::new(
+
self.parse_subexpr(self.dialect.prec_value(Precedence::UnaryPlusMinus))?,
+ ),
Review Comment:
You should keep `Precedence::PlusMinus` for `~` as well. PostgreSQL 17
returns -3 for `~ 1 * 2`, which is `~(1 * 2)`, and this head parses `(~ 1) *
2`. SQLite does bind `~` tighter, but that belongs in a separate change with
its own level.
```suggestion
expr:
Box::new(self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?),
```
##########
src/parser/mod.rs:
##########
@@ -2076,7 +2078,7 @@ impl<'a> Parser<'a> {
Ok(Expr::UnaryOp {
op,
expr: Box::new(
-
self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?,
+
self.parse_subexpr(self.dialect.prec_value(Precedence::UnaryPlusMinus))?,
Review Comment:
You should keep `Precedence::PlusMinus` for the geometric prefix operators
too, since they share the `Op` precedence of `|/`. This head parses `# 1 * 2`
as `(# 1) * 2`.
```suggestion
self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?,
```
##########
src/parser/mod.rs:
##########
@@ -2046,13 +2046,15 @@ impl<'a> Parser<'a> {
Ok(Expr::UnaryOp {
op,
expr: Box::new(
-
self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?,
+
self.parse_subexpr(self.dialect.prec_value(Precedence::UnaryPlusMinus))?,
Review Comment:
You should keep `Precedence::PlusMinus` here, because PostgreSQL gives these
prefix operators the `Op` precedence of gram.y, below `+`. This head parses `|/
25 * 4` as `(|/ 25) * 4`, while PostgreSQL 17 returns 10, the square root of
100.
```suggestion
self.parse_subexpr(self.dialect.prec_value(Precedence::PlusMinus))?,
```
##########
tests/sqlparser_postgres.rs:
##########
@@ -13154,3 +13154,66 @@ fn parse_alter_table_no_inherit() {
);
}
}
+
+#[test]
+fn parse_unary_plus_minus_before_pg_exponentiation() {
+ for (sql, unary_op, operand, exponent) in [
+ ("SELECT -1 ^ 0", UnaryOperator::Minus, "1", "0"),
+ ("SELECT +2 ^ 2", UnaryOperator::Plus, "2", "2"),
+ ] {
+ let select = pg().verified_only_select(sql);
+ let Expr::BinaryOp { left, op, right } =
expr_from_projection(&select.projection[0]) else {
+ panic!("Expected exponentiation expression")
+ };
+ assert_eq!(*op, BinaryOperator::PGExp);
+ assert_eq!(
+ right.as_ref(),
+ &Expr::Value(number(exponent).with_empty_span())
+ );
+ assert_eq!(
+ left.as_ref(),
+ &Expr::UnaryOp {
+ op: unary_op,
+ expr: Box::new(Expr::Value(number(operand).with_empty_span())),
+ }
+ );
+ }
+}
+
+#[test]
+fn parse_unary_minus_before_pg_at_time_zone() {
+ let select = pg().verified_only_select("SELECT -ts AT TIME ZONE 'UTC'");
+ let Expr::AtTimeZone { timestamp, .. } =
expr_from_projection(&select.projection[0]) else {
+ panic!("Expected AT TIME ZONE expression")
+ };
+ assert_eq!(
+ timestamp.as_ref(),
+ &Expr::UnaryOp {
+ op: UnaryOperator::Minus,
+ expr: Box::new(Expr::Identifier(Ident::new("ts"))),
+ }
+ );
+}
+
+#[test]
+fn parse_generic_unary_minus_before_caret() {
+ let statements = Parser::parse_sql(&GenericDialect {}, "SELECT -1 ^
0").unwrap();
+ let Statement::Query(query) = &statements[0] else {
+ panic!("Expected query")
+ };
+ let SetExpr::Select(select) = query.body.as_ref() else {
+ panic!("Expected select")
+ };
+ let Expr::BinaryOp { left, op, right } =
expr_from_projection(&select.projection[0]) else {
+ panic!("Expected bitwise expression")
+ };
+ assert_eq!(*op, BinaryOperator::BitwiseXor);
+ assert_eq!(
+ left.as_ref(),
+ &Expr::UnaryOp {
+ op: UnaryOperator::Minus,
+ expr: Box::new(Expr::Value(number("1").with_empty_span())),
+ }
+ );
+ assert_eq!(right.as_ref(), &Expr::Value(number("0").with_empty_span()));
+}
Review Comment:
Red test regarding the other changes suggested.
```suggestion
}
#[test]
fn parse_pg_prefix_operator_before_multiplication() {
for sql in ["SELECT ~ 1 * 2", "SELECT |/ 25 * 4", "SELECT # 1 * 2"] {
let select = pg().verified_only_select(sql);
let Expr::UnaryOp { expr, .. } =
expr_from_projection(&select.projection[0]) else {
panic!("Expected prefix operator expression in {sql}")
};
assert!(
matches!(
expr.as_ref(),
Expr::BinaryOp {
op: BinaryOperator::Multiply,
..
}
),
"{sql}"
);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
