diff --git a/ibis/backends/clickhouse/tests/snapshots/test_operators/test_binary_infix_parenthesization/lambda0/out.sql b/ibis/backends/clickhouse/tests/snapshots/test_operators/test_binary_infix_parenthesization/lambda0/out.sql index dbe622a75a1a..ea28d403dafd 100644 --- a/ibis/backends/clickhouse/tests/snapshots/test_operators/test_binary_infix_parenthesization/lambda0/out.sql +++ b/ibis/backends/clickhouse/tests/snapshots/test_operators/test_binary_infix_parenthesization/lambda0/out.sql @@ -1,5 +1,3 @@ SELECT - ( - "t0"."int_col" + "t0"."tinyint_col" - ) + "t0"."double_col" AS "Add(Add(int_col, tinyint_col), double_col)" + "t0"."int_col" + "t0"."tinyint_col" + "t0"."double_col" AS "Add(Add(int_col, tinyint_col), double_col)" FROM "functional_alltypes" AS "t0" \ No newline at end of file diff --git a/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out1.sql b/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out1.sql index a3b900a39f70..bce8cb2c918a 100644 --- a/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out1.sql +++ b/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out1.sql @@ -1,10 +1,6 @@ SELECT "t1"."key" AS "key", - SUM(( - ( - "t1"."value" + 1 - ) + 2 - ) + 3) AS "abc" + SUM("t1"."value" + 1 + 2 + 3) AS "abc" FROM ( SELECT * diff --git a/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out2.sql b/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out2.sql index 0c1b8f8a4a27..fa1932e2b1c5 100644 --- a/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out2.sql +++ b/ibis/backends/clickhouse/tests/snapshots/test_select/test_named_from_filter_groupby/out2.sql @@ -1,10 +1,6 @@ SELECT "t1"."key" AS "key", - SUM(( - ( - "t1"."value" + 1 - ) + 2 - ) + 3) AS "foo" + SUM("t1"."value" + 1 + 2 + 3) AS "foo" FROM ( SELECT * diff --git a/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/abc.sql b/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/abc.sql index c5e097cf1918..ced04c5596d0 100644 --- a/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/abc.sql +++ b/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/abc.sql @@ -1 +1 @@ -SELECT `t1`.`key`, SUM(((`t1`.`value` + 1) + 2) + 3) AS `abc` FROM (SELECT * FROM `t0` AS `t0` WHERE `t0`.`value` = 42) AS `t1` GROUP BY 1 \ No newline at end of file +SELECT `t1`.`key`, SUM(`t1`.`value` + 1 + 2 + 3) AS `abc` FROM (SELECT * FROM `t0` AS `t0` WHERE `t0`.`value` = 42) AS `t1` GROUP BY 1 \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/foo.sql b/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/foo.sql index a1218425e6e3..347ab11293cb 100644 --- a/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/foo.sql +++ b/ibis/backends/impala/tests/snapshots/test_exprs/test_named_from_filter_group_by/foo.sql @@ -1 +1 @@ -SELECT `t1`.`key`, SUM(((`t1`.`value` + 1) + 2) + 3) AS `foo` FROM (SELECT * FROM `t0` AS `t0` WHERE `t0`.`value` = 42) AS `t1` GROUP BY 1 \ No newline at end of file +SELECT `t1`.`key`, SUM(`t1`.`value` + 1 + 2 + 3) AS `foo` FROM (SELECT * FROM `t0` AS `t0` WHERE `t0`.`value` = 42) AS `t1` GROUP BY 1 \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_value_exprs/test_binary_infix_parenthesization/parens_left/out.sql b/ibis/backends/impala/tests/snapshots/test_value_exprs/test_binary_infix_parenthesization/parens_left/out.sql index 5129499fb406..d42802631f11 100644 --- a/ibis/backends/impala/tests/snapshots/test_value_exprs/test_binary_infix_parenthesization/parens_left/out.sql +++ b/ibis/backends/impala/tests/snapshots/test_value_exprs/test_binary_infix_parenthesization/parens_left/out.sql @@ -1,5 +1,3 @@ SELECT - ( - `t0`.`a` + `t0`.`b` - ) + `t0`.`c` AS `Add(Add(a, b), c)` + `t0`.`a` + `t0`.`b` + `t0`.`c` AS `Add(Add(a, b), c)` FROM `alltypes` AS `t0` \ No newline at end of file diff --git a/ibis/backends/sql/compilers/base.py b/ibis/backends/sql/compilers/base.py index 687b44b270f6..2b9031548c6e 100644 --- a/ibis/backends/sql/compilers/base.py +++ b/ibis/backends/sql/compilers/base.py @@ -239,20 +239,6 @@ def __getitem__(self, key: str) -> sge.Column: STAR = sge.Star() -def parenthesize_inputs(f): - """Decorate a translation rule to parenthesize inputs.""" - - def wrapper(self, op, *, left, right): - return f( - self, - op, - left=self._add_parens(op.left, left), - right=self._add_parens(op.right, right), - ) - - return wrapper - - @public class SQLGlotCompiler(abc.ABC): __slots__ = "f", "v" @@ -393,45 +379,50 @@ class SQLGlotCompiler(abc.ABC): ops.Uppercase: "upper", } - BINARY_INFIX_OPS = ( - # Binary operations - ops.Add, - ops.Subtract, - ops.Multiply, - ops.Divide, - ops.Modulus, - ops.Power, + BINARY_INFIX_OPS = { + # Numeric + ops.Add: (sge.Add, True), + ops.Subtract: (sge.Sub, False), + ops.Multiply: (sge.Mul, True), + ops.Divide: (sge.Div, False), + ops.Modulus: (sge.Mod, False), + ops.Power: (sge.Pow, False), # Comparisons - ops.GreaterEqual, - ops.Greater, - ops.LessEqual, - ops.Less, - ops.Equals, - ops.NotEquals, - # Boolean comparisons - ops.And, - ops.Or, - ops.Xor, - # Bitwise business - ops.BitwiseLeftShift, - ops.BitwiseRightShift, - ops.BitwiseAnd, - ops.BitwiseOr, - ops.BitwiseXor, - # Time arithmetic - ops.DateAdd, - ops.DateSub, - ops.DateDiff, - ops.TimestampAdd, - ops.TimestampSub, - ops.TimestampDiff, - # Interval Marginalia - ops.IntervalAdd, - ops.IntervalMultiply, - ops.IntervalSubtract, - ) + ops.GreaterEqual: (sge.GTE, False), + ops.Greater: (sge.GT, False), + ops.LessEqual: (sge.LTE, False), + ops.Less: (sge.LT, False), + ops.Equals: (sge.EQ, False), + ops.NotEquals: (sge.NEQ, False), + # Logical + ops.And: (sge.And, True), + ops.Or: (sge.Or, True), + ops.Xor: (sge.Xor, True), + # Bitwise + ops.BitwiseLeftShift: (sge.BitwiseLeftShift, False), + ops.BitwiseRightShift: (sge.BitwiseRightShift, False), + ops.BitwiseAnd: (sge.BitwiseAnd, True), + ops.BitwiseOr: (sge.BitwiseOr, True), + ops.BitwiseXor: (sge.BitwiseXor, True), + # Date + ops.DateAdd: (sge.Add, True), + ops.DateSub: (sge.Sub, False), + ops.DateDiff: (sge.Sub, False), + # Time + ops.TimeAdd: (sge.Add, True), + ops.TimeSub: (sge.Sub, False), + ops.TimeDiff: (sge.Sub, False), + # Timestamp + ops.TimestampAdd: (sge.Add, True), + ops.TimestampSub: (sge.Sub, False), + ops.TimestampDiff: (sge.Sub, False), + # Interval + ops.IntervalAdd: (sge.Add, True), + ops.IntervalMultiply: (sge.Mul, True), + ops.IntervalSubtract: (sge.Sub, False), + } - NEEDS_PARENS = BINARY_INFIX_OPS + (ops.IsNull,) + NEEDS_PARENS = tuple(BINARY_INFIX_OPS) + (ops.IsNull,) # Constructed dynamically in `__init_subclass__` from their respective # UPPERCASE values to handle inheritance, do not modify directly here. @@ -469,6 +460,19 @@ def impl(self, _, *, _name: str = target_name, **kw): for op, target_name in cls.SIMPLE_OPS.items(): setattr(cls, methodname(op), make_impl(op, target_name)) + # Define binary op methods, only if BINARY_INFIX_OPS is set on the + # compiler class. + if binops := cls.__dict__.get("BINARY_INFIX_OPS", {}): + + def make_binop(sge_cls, associative): + def impl(self, op, *, left, right): + return self.binop(sge_cls, op, left, right, associative=associative) + + return impl + + for op, (sge_cls, associative) in binops.items(): + setattr(cls, methodname(op), make_binop(sge_cls, associative)) + # unconditionally raise an exception for unsupported operations # # these *must* be defined after SIMPLE_OPS to handle compilers that @@ -1505,93 +1509,16 @@ def visit_SQLQueryResult(self, op, *, query, schema, source): def visit_RegexExtract(self, op, *, arg, pattern, index): return self.f.regexp_extract(arg, pattern, index, dialect=self.dialect) - @parenthesize_inputs - def visit_Add(self, op, *, left, right): - return sge.Add(this=left, expression=right) - - visit_DateAdd = visit_TimestampAdd = visit_IntervalAdd = visit_Add - - @parenthesize_inputs - def visit_Subtract(self, op, *, left, right): - return sge.Sub(this=left, expression=right) - - visit_DateSub = visit_DateDiff = visit_TimestampSub = visit_TimestampDiff = ( - visit_IntervalSubtract - ) = visit_Subtract - - @parenthesize_inputs - def visit_Multiply(self, op, *, left, right): - return sge.Mul(this=left, expression=right) - - visit_IntervalMultiply = visit_Multiply - - @parenthesize_inputs - def visit_Divide(self, op, *, left, right): - return sge.Div(this=left, expression=right) - - @parenthesize_inputs - def visit_Modulus(self, op, *, left, right): - return sge.Mod(this=left, expression=right) - - @parenthesize_inputs - def visit_Power(self, op, *, left, right): - return sge.Pow(this=left, expression=right) - - @parenthesize_inputs - def visit_GreaterEqual(self, op, *, left, right): - return sge.GTE(this=left, expression=right) - - @parenthesize_inputs - def visit_Greater(self, op, *, left, right): - return sge.GT(this=left, expression=right) - - @parenthesize_inputs - def visit_LessEqual(self, op, *, left, right): - return sge.LTE(this=left, expression=right) - - @parenthesize_inputs - def visit_Less(self, op, *, left, right): - return sge.LT(this=left, expression=right) - - @parenthesize_inputs - def visit_Equals(self, op, *, left, right): - return sge.EQ(this=left, expression=right) - - @parenthesize_inputs - def visit_NotEquals(self, op, *, left, right): - return sge.NEQ(this=left, expression=right) - - @parenthesize_inputs - def visit_And(self, op, *, left, right): - return sge.And(this=left, expression=right) - - @parenthesize_inputs - def visit_Or(self, op, *, left, right): - return sge.Or(this=left, expression=right) - - @parenthesize_inputs - def visit_Xor(self, op, *, left, right): - return sge.Xor(this=left, expression=right) - - @parenthesize_inputs - def visit_BitwiseLeftShift(self, op, *, left, right): - return sge.BitwiseLeftShift(this=left, expression=right) - - @parenthesize_inputs - def visit_BitwiseRightShift(self, op, *, left, right): - return sge.BitwiseRightShift(this=left, expression=right) - - @parenthesize_inputs - def visit_BitwiseAnd(self, op, *, left, right): - return sge.BitwiseAnd(this=left, expression=right) - - @parenthesize_inputs - def visit_BitwiseOr(self, op, *, left, right): - return sge.BitwiseOr(this=left, expression=right) - - @parenthesize_inputs - def visit_BitwiseXor(self, op, *, left, right): - return sge.BitwiseXor(this=left, expression=right) + def binop(self, sg_expr, op, left, right, *, associative=False): + # If the op is associative we can skip parenthesizing ops of the same + # type if they're on the left, since they would evaluate the same. + # SQLGlot has an optimizer for generating long sql chains of the same + # op of this form without recursion, by avoiding parenthesis in this + # common case we can make use of this optimization to handle large + # operator chains. + if not associative or type(op) is not type(op.left): + left = self._add_parens(op.left, left) + return sg_expr(this=left, expression=self._add_parens(op.right, right)) def visit_Undefined(self, op, **_): raise com.OperationNotDefinedError( diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-dateintervalD/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-dateintervalD/out.sql new file mode 100644 index 000000000000..5dc42e651bf3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-dateintervalD/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" + "t0"."b" + "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" + ( + "t0"."b" + "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-int8/out.sql new file mode 100644 index 000000000000..5dc42e651bf3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-int8/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" + "t0"."b" + "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" + ( + "t0"."b" + "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-interval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-interval/out.sql new file mode 100644 index 000000000000..5dc42e651bf3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-interval/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" + "t0"."b" + "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" + ( + "t0"."b" + "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timeinterval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timeinterval/out.sql new file mode 100644 index 000000000000..5dc42e651bf3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timeinterval/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" + "t0"."b" + "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" + ( + "t0"."b" + "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timestampinterval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timestampinterval/out.sql new file mode 100644 index 000000000000..5dc42e651bf3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/add-timestampinterval/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" + "t0"."b" + "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" + ( + "t0"."b" + "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-bool/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-bool/out.sql new file mode 100644 index 000000000000..b516c992a3d7 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-bool/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" AND "t0"."b" AND "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" AND ( + "t0"."b" AND "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-int8/out.sql new file mode 100644 index 000000000000..b6073fe25b30 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/and_-int8/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" & "t0"."b" & "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" & ( + "t0"."b" & "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/lshift-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/lshift-int8/out.sql new file mode 100644 index 000000000000..c95b7e0981e6 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/lshift-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" << "t0"."b" + ) << "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" << ( + "t0"."b" << "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mod-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mod-int8/out.sql new file mode 100644 index 000000000000..1b905f606640 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mod-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" % "t0"."b" + ) % "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" % ( + "t0"."b" % "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-int8/out.sql new file mode 100644 index 000000000000..23229e9a74f3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-int8/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" * "t0"."b" * "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" * ( + "t0"."b" * "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-intervalint8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-intervalint8/out.sql new file mode 100644 index 000000000000..23229e9a74f3 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/mul-intervalint8/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" * "t0"."b" * "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" * ( + "t0"."b" * "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-bool/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-bool/out.sql new file mode 100644 index 000000000000..d7e389da152b --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-bool/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" OR "t0"."b" OR "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" OR ( + "t0"."b" OR "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-int8/out.sql new file mode 100644 index 000000000000..8405cf7265a5 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/or_-int8/out.sql @@ -0,0 +1,8 @@ +SELECT + "t0"."a" | "t0"."b" | "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" | ( + "t0"."b" | "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/pow-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/pow-int8/out.sql new file mode 100644 index 000000000000..8021afa46650 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/pow-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + POWER(( + POWER("t0"."a", "t0"."b") + ), "t0"."c") AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + POWER("t0"."a", ( + POWER("t0"."b", "t0"."c") + )) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/rshift-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/rshift-int8/out.sql new file mode 100644 index 000000000000..6daf9292c1e1 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/rshift-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" >> "t0"."b" + ) >> "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" >> ( + "t0"."b" >> "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-dateintervalD/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-dateintervalD/out.sql new file mode 100644 index 000000000000..5f0054cd14f8 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-dateintervalD/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" - "t0"."b" + ) - "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" - ( + "t0"."b" - "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-int8/out.sql new file mode 100644 index 000000000000..5f0054cd14f8 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" - "t0"."b" + ) - "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" - ( + "t0"."b" - "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-interval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-interval/out.sql new file mode 100644 index 000000000000..5f0054cd14f8 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-interval/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" - "t0"."b" + ) - "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" - ( + "t0"."b" - "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timeinterval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timeinterval/out.sql new file mode 100644 index 000000000000..5f0054cd14f8 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timeinterval/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" - "t0"."b" + ) - "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" - ( + "t0"."b" - "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timestampinterval/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timestampinterval/out.sql new file mode 100644 index 000000000000..5f0054cd14f8 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/sub-timestampinterval/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" - "t0"."b" + ) - "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" - ( + "t0"."b" - "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/truediv-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/truediv-int8/out.sql new file mode 100644 index 000000000000..9ceccb944377 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/truediv-int8/out.sql @@ -0,0 +1,10 @@ +SELECT + ( + "t0"."a" / "t0"."b" + ) / "t0"."c" AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + "t0"."a" / ( + "t0"."b" / "t0"."c" + ) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-bool/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-bool/out.sql new file mode 100644 index 000000000000..3d255e484c3a --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-bool/out.sql @@ -0,0 +1,10 @@ +SELECT + (("t0"."a" AND (NOT "t0"."b")) OR ((NOT "t0"."a") AND "t0"."b") AND (NOT "t0"."c")) OR ((NOT ("t0"."a" AND (NOT "t0"."b")) OR ((NOT "t0"."a") AND "t0"."b")) AND "t0"."c") AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + ("t0"."a" AND (NOT ( + ("t0"."b" AND (NOT "t0"."c")) OR ((NOT "t0"."b") AND "t0"."c") + ))) OR ((NOT "t0"."a") AND ( + ("t0"."b" AND (NOT "t0"."c")) OR ((NOT "t0"."b") AND "t0"."c") + )) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-int8/out.sql b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-int8/out.sql new file mode 100644 index 000000000000..ae9cd6f62f86 --- /dev/null +++ b/ibis/backends/tests/sql/snapshots/test_sql/test_binop_parens/xor-int8/out.sql @@ -0,0 +1,8 @@ +SELECT + XOR(XOR("t0"."a", "t0"."b"), "t0"."c") AS "x" +FROM "t" AS "t0" --- op(op(a, b), c); +SELECT + XOR("t0"."a", ( + XOR("t0"."b", "t0"."c") + )) AS "x" +FROM "t" AS "t0" --- op(a, op(b, c)); diff --git a/ibis/backends/tests/sql/test_sql.py b/ibis/backends/tests/sql/test_sql.py index 00aac59d5771..01fd47220982 100644 --- a/ibis/backends/tests/sql/test_sql.py +++ b/ibis/backends/tests/sql/test_sql.py @@ -89,6 +89,60 @@ def test_comparisons(functional_alltypes, opname, snapshot): snapshot.assert_match(to_sql(expr), "out.sql") +@pytest.mark.parametrize( + "opname, dtype, associative", + [ + param(*v, id=f"{v[0]}-{v[1]}") + for v in [ + # Numeric + ("add", "int8", True), + ("mul", "int8", True), + ("sub", "int8", False), + ("truediv", "int8", False), + ("mod", "int8", False), + ("pow", "int8", False), + ("and_", "int8", True), + ("or_", "int8", True), + ("xor", "int8", True), + ("lshift", "int8", False), + ("rshift", "int8", False), + # Logical + ("and_", "bool", True), + ("or_", "bool", True), + ("xor", "bool", True), + # Date + ("add", "date,interval('D')", True), + ("sub", "date,interval('D')", False), + # Time + ("add", "time,interval", True), + ("sub", "time,interval", False), + # Timestamp + ("add", "timestamp,interval", True), + ("sub", "timestamp,interval", False), + # Interval + ("add", "interval", True), + ("sub", "interval", False), + ("mul", "interval,int8", True), + ] + ], +) +def test_binop_parens(snapshot, opname, dtype, associative): + op = getattr(operator, opname) + dtypes = [ibis.dtype(d) for d in dtype.split(",")] + while len(dtypes) < 3: + dtypes.append(dtypes[-1]) # repeat last dtype + + t = ibis.table(dict(zip("abc", dtypes)), name="t") + + expr1 = op(op(t.a, t.b), t.c).name("x") + expr2 = op(t.a, op(t.b, t.c)).name("x") + sql1 = to_sql(expr1) + sql2 = to_sql(expr2) + assert sql1 != sql2 + combined = f"{sql1} --- op(op(a, b), c);\n{sql2} --- op(a, op(b, c));\n" + snapshot.assert_match(combined, "out.sql") + + @pytest.mark.parametrize( "expr_fn", [