Skip to content

Commit 43c2e35

Browse files
fix comparison op issues
1 parent 6ef38f3 commit 43c2e35

3 files changed

Lines changed: 68 additions & 7 deletions

File tree

packages/bigframes/bigframes/core/compile/lowering.py

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,47 @@ def _lower_cast(cast_op: ops.AsTypeOp, arg: expression.Expression):
463463
return cast_op.as_expr(arg)
464464

465465

466-
LOWER_COMPARISONS = tuple(
466+
class LowerEqNullsMatchRule(op_lowering.OpLoweringRule):
467+
@property
468+
def op(self) -> type[ops.ScalarOp]:
469+
return comparison_ops.EqNullsMatchOp
470+
471+
def lower(self, expr: expression.OpExpression) -> expression.Expression:
472+
assert isinstance(expr.op, comparison_ops.EqNullsMatchOp)
473+
arg1, arg2 = _coerce_comparables(expr.children[0], expr.children[1])
474+
475+
# True constant
476+
true_const = expression.const(True)
477+
# False constant
478+
false_const = expression.const(False)
479+
480+
# equal = arg1 == arg2
481+
equal_expr = ops.eq_op.as_expr(arg1, arg2)
482+
483+
# isnull1 = arg1.isnull()
484+
isnull1_expr = ops.isnull_op.as_expr(arg1)
485+
486+
# isnull2 = arg2.isnull()
487+
isnull2_expr = ops.isnull_op.as_expr(arg2)
488+
489+
# both_null = isnull1 & isnull2
490+
both_null_expr = ops.and_op.as_expr(isnull1_expr, isnull2_expr)
491+
492+
# any_null = isnull1 | isnull2
493+
any_null_expr = ops.or_op.as_expr(isnull1_expr, isnull2_expr)
494+
495+
# inner_where = where(false, any_null, equal)
496+
inner_where_expr = ops.where_op.as_expr(false_const, any_null_expr, equal_expr)
497+
498+
# outer_where = where(true, both_null, inner_where)
499+
null_safe_eq_expr = ops.where_op.as_expr(
500+
true_const, both_null_expr, inner_where_expr
501+
)
502+
503+
return null_safe_eq_expr
504+
505+
506+
POLARS_LOWER_COMPARISONS = tuple(
467507
CoerceArgsRule(op)
468508
for op in (
469509
comparison_ops.EqOp,
@@ -476,8 +516,20 @@ def _lower_cast(cast_op: ops.AsTypeOp, arg: expression.Expression):
476516
)
477517
)
478518

519+
SUBSTRAIT_LOWER_COMPARISONS = tuple(
520+
CoerceArgsRule(op)
521+
for op in (
522+
comparison_ops.EqOp,
523+
comparison_ops.NeOp,
524+
comparison_ops.LtOp,
525+
comparison_ops.GtOp,
526+
comparison_ops.LeOp,
527+
comparison_ops.GeOp,
528+
)
529+
)
530+
479531
POLARS_LOWERING_RULES = (
480-
*LOWER_COMPARISONS,
532+
*POLARS_LOWER_COMPARISONS,
481533
LowerAddRule(),
482534
LowerSubRule(),
483535
LowerMulRule(),
@@ -492,7 +544,10 @@ def _lower_cast(cast_op: ops.AsTypeOp, arg: expression.Expression):
492544
LowerFloorOp(),
493545
)
494546

495-
SUBSTRAIT_LOWERING_RULES = (*LOWER_COMPARISONS,)
547+
SUBSTRAIT_LOWERING_RULES = (
548+
LowerEqNullsMatchRule(),
549+
*SUBSTRAIT_LOWER_COMPARISONS,
550+
)
496551

497552

498553
def lower_ops_to_polars(root: bigframe_node.BigFrameNode) -> bigframe_node.BigFrameNode:

packages/bigframes/bigframes/core/compile/substrait/compiler.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,13 @@ def _compile_deref(
606606
def _compile_op_expr(
607607
self, expr: ex.OpExpression, child: nodes.BigFrameNode
608608
) -> algebra_pb2.Expression:
609-
return self._compile_op(expr.op, expr.inputs, child)
609+
pb_expr = self._compile_op(expr.op, expr.inputs, child)
610+
if pb_expr.HasField("scalar_function"):
611+
if not pb_expr.scalar_function.HasField("output_type"):
612+
output_dtype = self._get_expression_dtype(expr, child)
613+
type_dict = self._convert_type(output_dtype)
614+
json_format.ParseDict(type_dict, pb_expr.scalar_function.output_type)
615+
return pb_expr
610616

611617
@singledispatchmethod
612618
def _compile_op(
@@ -677,7 +683,9 @@ def _compile_cast(
677683
json_format.ParseDict(type_dict, cast.type)
678684

679685
# alternative: FAILURE_BEHAVIOR_RETURN_NULL not supported by acero
680-
cast.failure_behavior = algebra_pb2.Expression.Cast.FAILURE_BEHAVIOR_THROW_EXCEPTION
686+
cast.failure_behavior = (
687+
algebra_pb2.Expression.Cast.FAILURE_BEHAVIOR_THROW_EXCEPTION
688+
)
681689
return pb_expr
682690

683691
def _get_expression_dtype(

packages/bigframes/tests/system/small/engines/test_numeric_ops.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ def test_engines_project_floor(
100100
"polars",
101101
"bq",
102102
"bq-sqlglot",
103-
"substrait-datafusion",
104-
"substrait-acero",
105103
],
106104
indirect=True,
107105
)

0 commit comments

Comments
 (0)