Skip to content
Open
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
2 changes: 1 addition & 1 deletion docs/source/contributor-guide/adding_a_new_expression.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ When the return type is set on the proto, the native planner skips the registry

#### Arguments that Spark skips after a NULL

A null-intolerant `BinaryExpression` or `TernaryExpression` returns NULL as soon as one of its arguments is NULL, without evaluating the arguments after it. Native execution evaluates every argument of a `ScalarFunc` over the whole batch, so an argument that can fail, such as an ANSI cast of a malformed string, would fail on rows that Spark skips. Wrap the serialized call of such an expression in `withNullShortCircuit`, as `CometArrayContains` and `CometSlice` do, after checking the conditions in its Scaladoc.
A null-intolerant `BinaryExpression` or `TernaryExpression` returns NULL as soon as one of its arguments is NULL, without evaluating the arguments after it. Native execution evaluates every argument of a `ScalarFunc` over the whole batch, so an argument that can fail, such as an ANSI cast of a malformed string, would fail on rows that Spark skips. Wrap the serialized call of such an expression in `withNullShortCircuit`, as `CometArrayContains` and `CometSlice` do, after checking the conditions in its Scaladoc. `CometConcat` wraps `concat` the same way. Arithmetic is a `MathExpr` rather than a `ScalarFunc`, so `createMathExpression` sets `MathExpr.null_short_circuit` instead, which also evaluates the divisor of `/`, `DIV` and `%` first, as Spark does.

#### Registering the Expression Handler

Expand Down
8 changes: 7 additions & 1 deletion native/core/src/execution/expressions/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,17 @@ macro_rules! arithmetic_expr_builder {
let expr = $crate::extract_expr!(spark_expr, $expr_type);
let eval_mode =
$crate::execution::planner::from_protobuf_eval_mode(expr.eval_mode)?;
planner.create_binary_expr(
planner.create_binary_expr_with_options(
spark_expr, // Pass the full spark_expr for query_context lookup
expr.left.as_ref().unwrap(),
expr.right.as_ref().unwrap(),
expr.return_type.as_ref(),
$operator,
input_schema,
$crate::execution::planner::BinaryExprOptions {
null_short_circuit: expr.null_short_circuit,
..Default::default()
},
eval_mode,
)
}
Expand Down Expand Up @@ -105,6 +109,7 @@ impl ExpressionBuilder for IntegralDivideBuilder {
BinaryExprOptions {
is_integral_div: true,
check_divide_overflow: expr.check_divide_overflow,
null_short_circuit: expr.null_short_circuit,
},
eval_mode,
)
Expand Down Expand Up @@ -135,6 +140,7 @@ impl ExpressionBuilder for RemainderBuilder {
.unwrap(),
input_schema,
eval_mode == EvalMode::Ansi,
expr.null_short_circuit,
&planner.session_ctx().state(),
);
result.map_err(|e| ExecutionError::GeneralError(e.to_string()))
Expand Down
Loading