Fix Numeric signature coercion to properly handle null types - #24988
Fix Numeric signature coercion to properly handle null types#24988Jefffrey wants to merge 3 commits into
Numeric signature coercion to properly handle null types#24988Conversation
| // 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(); |
There was a problem hiding this comment.
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
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)
| // 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 { |
There was a problem hiding this comment.
just moved this up; this would only happen if all input types were null type
| query I | ||
| SELECT MOD(NULL, 3); | ||
| ---- | ||
| NULL |
There was a problem hiding this comment.
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);
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24988 +/- ##
========================================
Coverage 81.67% 81.67%
========================================
Files 1126 1126
Lines 414524 414782 +258
Branches 414524 414782 +258
========================================
+ Hits 338558 338780 +222
- Misses 56054 56070 +16
- Partials 19912 19932 +20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Which issue does this PR close?
Rationale for this change
For UDFs using
Numericsignature with more than one argument (currently mod & pmod from datafusion-spark), they cannot handle inputsNull, Intwhereas they acceptInt, Null. Fixing the coercion logic to ensure both are accepted.What changes are included in this PR?
Modify numeric type coercion handling to ignore null types
What is the testing strategy for this PR?
Added unit & SLT tests
Are there any user-facing changes?
No