Skip to content

Fix Numeric signature coercion to properly handle null types #24988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jefffrey wants to merge 6 commits into
base: main
Choose a base branch
from
+54 −14
Open

Fix Numeric signature coercion to properly handle null types #24988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e87d45
Fix `Numeric` signature coercion to properly handle null types
Jefffrey Sep 6, 2026
f3b9eb9
fix test
Jefffrey Sep 6, 2026
cd6278b
fix test
Jefffrey Sep 6, 2026
eec8f6c
Merge branch 'main' into numeric-null-coercion
Jefffrey Sep 7, 2026
93354e5
fix test
Jefffrey Sep 7, 2026
913c8c6
Merge branch 'main' into numeric-null-coercion
Jefffrey Sep 7, 2026
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
47 changes: 36 additions & 11 deletions datafusion/expr/src/type_coercion/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,13 +816,21 @@ fn get_valid_types(
TypeSignature::Numeric(number) => {
function_length_check(function_name, current_types.len(), *number)?;

let non_nulls = current_types
.iter()
.filter(|&t| NativeType::from(t) != NativeType::Null)
.collect::<Vec<_>>();
let mut valid_type = non_nulls
.first()
.copied()
.cloned()
// Fallback to default type if we don't know which type to coerced to
// f64 is chosen since most of the math functions utilize Signature::numeric,
// and their default type is double precision
.unwrap_or(DataType::Float64);
// Find common numeric type among given types except string
let mut valid_type = current_types.first().unwrap().to_owned();

@Jefffrey Jefffrey Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if first argument was null type, valid_type is set to it, and below when we call binary_numeric_coercion it'll fail since it expects two numeric type arguments:

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

Lines 1047 to 1054 in 5b389eb

/// Coerce `lhs_type` and `rhs_type` to a common type where both are numeric
pub fn binary_numeric_coercion(
lhs_type: &DataType,
rhs_type: &DataType,
) -> Option<DataType> {
if !lhs_type.is_numeric() || !rhs_type.is_numeric() {
return None;
}

rather than fixing inside binary_numeric_coercion, decided it should be better to ensure we ignore null types initially, then run coercion on any non-null type, which is essentially what was happening in main (so long as the first type wasnt null)

for t in current_types.iter().skip(1) {
for &t in non_nulls.iter().skip(1) {
let logical_data_type: NativeType = t.into();
if logical_data_type == NativeType::Null {
continue;
}

if !logical_data_type.is_numeric() {
return plan_err!(
Expand All @@ -840,12 +848,7 @@ fn get_valid_types(
}

let logical_data_type: NativeType = valid_type.clone().into();
// Fallback to default type if we don't know which type to coerced to
// f64 is chosen since most of the math functions utilize Signature::numeric,
// and their default type is double precision
if logical_data_type == NativeType::Null {

@Jefffrey Jefffrey Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just moved this up; this would only happen if all input types were null type

valid_type = DataType::Float64;
} else if !logical_data_type.is_numeric() {
if !logical_data_type.is_numeric() {
return plan_err!(
"Function '{function_name}' expects Numeric but received {logical_data_type}"
);
Expand Down Expand Up @@ -1451,6 +1454,13 @@ mod tests {
);
assert_eq!(got, [DataType::Float64]);

let got = get_valid_types_flatten(
"test",
&TypeSignature::Numeric(2),
&[DataType::Null, DataType::Null],
);
assert_eq!(got, [DataType::Float64, DataType::Float64]);

// Rejects non-numeric arg.
let got = get_valid_types(
"test",
Expand All @@ -1463,6 +1473,21 @@ mod tests {
"Function 'test' expects Numeric but received Timestamp(s)"
);

// Nulls should get ignored among other valid types
let got = get_valid_types_flatten(
"test",
&TypeSignature::Numeric(2),
&[DataType::Null, DataType::Int32],
);
assert_eq!(got, [DataType::Int32, DataType::Int32]);

let got = get_valid_types_flatten(
"test",
&TypeSignature::Numeric(2),
&[DataType::Int32, DataType::Null],
);
assert_eq!(got, [DataType::Int32, DataType::Int32]);

Ok(())
}

Expand Down
15 changes: 15 additions & 0 deletions datafusion/sqllogictest/test_files/spark/math/mod.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,21 @@ SELECT MOD(NULL::int, NULL::int) as mod_null_3;
----
NULL

query I
SELECT MOD(NULL, 3);
----
NULL

@Jefffrey Jefffrey Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently failing on main:

1. query failed: DataFusion error: Error during planning: For function 'mod' Null and Int64 are not coercible to a common numeric type. No function matches the given name and argument types 'mod(Null, Int64)'. You might need to add explicit type casts.
        Candidate functions:
        mod(Numeric(2))
[SQL] SELECT MOD(NULL, 3);


query I
SELECT MOD(10, NULL);
----
NULL

query R
SELECT MOD(NULL, NULL);
----
NULL

# Special values: NaN and Infinity
query R
SELECT MOD(5.0::float8, 'NaN'::float8) as mod_nan_1;
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sqllogictest/test_files/spark/math/pmod.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ SELECT arrow_typeof(pmod(2.5::decimal(3,1), NULL));
----
Decimal128(3, 1)

# An untyped NULL beside a typed non-decimal argument takes the Numeric path,
# which cannot coerce the pair. `mod` rejects it the same way.
statement error DataFusion error: Error during planning: Internal error: Function 'pmod' failed to match any signature
query I
SELECT pmod(NULL, 3::int);
----
NULL

# PMOD tests with large integers
query I
Expand Down
Loading