Refactor power() signature away from user defined
#18968
Conversation
github-actions
bot
added
sqllogictest
| fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> { | ||
| Ok(arg_types[0].clone()) | ||
| if arg_types[0].is_null() { | ||
| Ok(DataType::Int64) |
There was a problem hiding this comment.
Maintaining existing behaviour for null types
| // Null propagation | ||
| if base_type.is_null() || exponent_type.is_null() { | ||
| let return_type = self.return_type(&[base_type, exponent_type])?; | ||
| return Ok(ExprSimplifyResult::Simplified(lit( | ||
| ScalarValue::Null.cast_to(&return_type)? | ||
| ))); | ||
| } |
There was a problem hiding this comment.
Similar to what we do for log:
datafusion/datafusion/functions/src/math/log.rs
Lines 278 to 283 in c6f7363
| } | ||
|
|
||
| #[test] | ||
| fn test_power_f64() { |
There was a problem hiding this comment.
Moved these to SLTs
| (NULL, NULL, NULL, NULL); | ||
|
|
||
| query IIRRIR rowsort | ||
| query IRRRIR rowsort |
There was a problem hiding this comment.
This is a minor difference; previously we were able to cast floats to integer, but with the new signature we fallback to computing as float if either base or exponent is float.
| vec![ | ||
| TypeSignature::Coercible(vec![integer.clone(), integer.clone()]), | ||
| TypeSignature::Coercible(vec![decimal.clone(), integer.clone()]), | ||
| TypeSignature::Coercible(vec![decimal.clone(), float.clone()]), |
There was a problem hiding this comment.
How does this work for power(base::decimal(38, 0), exponent::decimal(38, 0)) ?
I see this test case below but I don't see where the exponent is casted to a Float64.
In the old code it was done at https://github.com/apache/datafusion/pull/18968/files#diff-598356e1af0ad23287c968ebf6310366e5127b91a037e076bc264dac3a55e768L192
There was a problem hiding this comment.
The exponent would get case to float by the coercible signature, as float there accepts any numeric as input type
let float = Coercion::new_implicit(
TypeSignatureClass::Native(logical_float64()),
vec![TypeSignatureClass::Numeric],
NativeType::Float64,
);2 participants
Which issue does this PR close?
Signature::user_definedfor normal functions #12725Rationale for this change
Prefer to avoid user_defined for consistency in function definitions.
What changes are included in this PR?
Refactor signature of power away from user_defined.
Are these changes tested?
Existing tests.
Are there any user-facing changes?
No.