Skip to content

Commit cc0e451

Browse files
fix comparison op issues
1 parent 50effa9 commit cc0e451

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
@@ -459,7 +459,47 @@ def _lower_cast(cast_op: ops.AsTypeOp, arg: expression.Expression):
459459
return cast_op.as_expr(arg)
460460

461461

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

515+
SUBSTRAIT_LOWER_COMPARISONS = tuple(
516+
CoerceArgsRule(op)
517+
for op in (
518+
comparison_ops.EqOp,
519+
comparison_ops.NeOp,
520+
comparison_ops.LtOp,
521+
comparison_ops.GtOp,
522+
comparison_ops.LeOp,
523+
comparison_ops.GeOp,
524+
)
525+
)
526+
475527
POLARS_LOWERING_RULES = (
476-
*LOWER_COMPARISONS,
528+
*POLARS_LOWER_COMPARISONS,
477529
LowerAddRule(),
478530
LowerSubRule(),
479531
LowerMulRule(),
@@ -488,7 +540,10 @@ def _lower_cast(cast_op: ops.AsTypeOp, arg: expression.Expression):
488540
LowerFloorOp(),
489541
)
490542

491-
SUBSTRAIT_LOWERING_RULES = (*LOWER_COMPARISONS,)
543+
SUBSTRAIT_LOWERING_RULES = (
544+
LowerEqNullsMatchRule(),
545+
*SUBSTRAIT_LOWER_COMPARISONS,
546+
)
492547

493548

494549
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)