Skip to content

Commit 30efc4d

Browse files
authored
Fix NULL Arithmetic Handling for Numerical Operators in Type Coercion (#17418)
* fix: modify the type coercion logic to avoid planning error * fix: move logic for both nulls to signature method * fix: handle separately only arithmetics * fix: minor cleanup, explanation and a test * fix: address CR
1 parent 14a7ade commit 30efc4d

File tree

2 files changed

+22
-6
lines changed

2 files changed

+22
-6
lines changed

datafusion/expr-common/src/type_coercion/binary.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,24 @@ impl<'a> BinaryTypeCoercer<'a> {
124124

125125
/// Returns a [`Signature`] for applying `op` to arguments of type `lhs` and `rhs`
126126
fn signature(&'a self) -> Result<Signature> {
127+
// Special handling for arithmetic operations with both `lhs` and `rhs` NULL:
128+
// When both operands are NULL, we are providing a concrete numeric type (Int64)
129+
// to allow the arithmetic operation to proceed. This ensures NULL `op` NULL returns NULL
130+
// instead of failing during planning.
131+
if matches!((self.lhs, self.rhs), (DataType::Null, DataType::Null))
132+
&& self.op.is_numerical_operators()
133+
{
134+
return Ok(Signature::uniform(DataType::Int64));
135+
}
136+
127137
if let Some(coerced) = null_coercion(self.lhs, self.rhs) {
128-
use Operator::*;
129138
// Special handling for arithmetic + null coercion:
130139
// For arithmetic operators on non-temporal types, we must handle the result type here using Arrow's numeric kernel.
131140
// This is because Arrow expects concrete numeric types, and this ensures the correct result type (e.g., for NULL + Int32, result is Int32).
132141
// For all other cases (including temporal arithmetic and non-arithmetic operators),
133142
// we can delegate to signature_inner(&coerced, &coerced), which handles the necessary logic for those operators.
134143
// In those cases, signature_inner is designed to work with the coerced type, even if it originated from a NULL.
135-
if matches!(self.op, Plus | Minus | Multiply | Divide | Modulo)
136-
&& !coerced.is_temporal()
137-
{
144+
if self.op.is_numerical_operators() && !coerced.is_temporal() {
138145
let ret = self.get_result(&coerced, &coerced).map_err(|e| {
139146
plan_datafusion_err!(
140147
"Cannot get result type for arithmetic operation {coerced} {} {coerced}: {e}",
@@ -1528,8 +1535,8 @@ fn timeunit_coercion(lhs_unit: &TimeUnit, rhs_unit: &TimeUnit) -> TimeUnit {
15281535
}
15291536
}
15301537

1531-
/// Coercion rules from NULL type. Since NULL can be casted to any other type in arrow,
1532-
/// either lhs or rhs is NULL, if NULL can be casted to type of the other side, the coercion is valid.
1538+
/// Coercion rules from NULL type. Since NULL can be cast to any other type in arrow,
1539+
/// either lhs or rhs is NULL, if NULL can be cast to type of the other side, the coercion is valid.
15331540
fn null_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
15341541
match (lhs_type, rhs_type) {
15351542
(DataType::Null, other_type) | (other_type, DataType::Null) => {

datafusion/sqllogictest/test_files/operator.slt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,15 @@ from numeric_types;
262262
----
263263
Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64
264264

265+
############### NULL arithmetic ###############
266+
267+
# select both nulls with basic arithmetic operations
268+
query IIIII
269+
select null + null, null - null, null * null, null / null, null % null;
270+
----
271+
NULL NULL NULL NULL NULL
272+
273+
265274
###############
266275
# Test for comparison with constants uses efficient types
267276
# Expect the physical plans to compare with constants of the same type

0 commit comments

Comments
 (0)