This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24282-b551ba03753d719ff83a14b7f8f36bf667cdee67 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 3359eab09e634bf8298d3edbd2906c042c61791c Author: Jamison-J <[email protected]> AuthorDate: Sun Aug 16 07:07:40 2026 +0000 feat: add implicit native coercion constructor (#24282) ## Which issue does this PR close? - Closes #24135. ## Rationale for this change When a coercion's desired type is a native logical type, callers currently have to repeat that type as both `TypeSignatureClass::Native(...)` and `default_casted_type`. This duplication is confusing and can allow the two values to disagree. ## What changes are included in this PR? - Add `Coercion::new_implicit_native`, which derives `default_casted_type` from the desired logical type and delegates to the existing `new_implicit` constructor. - Keep `Coercion::new_implicit` unchanged for backward compatibility and non-native desired type classes. - Migrate the representative `reverse` and `approx_percentile_cont` call sites linked from the issue. - Update the `Coercion` rustdoc example and add a constructor-equivalence unit test. ## Are these changes tested? Yes: - `cargo test -p datafusion-expr-common test_new_implicit_native --lib` - `cargo test -p datafusion-functions reverse --lib` - `cargo test -p datafusion-functions-aggregate approx_percentile_cont --lib` - `cargo test -p datafusion-expr-common --doc` - `cargo clippy -p datafusion-expr-common -p datafusion-functions -p datafusion-functions-aggregate --all-targets --all-features -- -D warnings` - `cargo fmt --all` The required full-workspace `cargo clippy --all-targets --all-features -- -D warnings` was also attempted on Windows. It reached an unrelated MSVC native-link failure while building `protobuf-src` (`LNK2005`/`LNK4098` in `protoc-gen-upb`); affected-crate clippy passed with warnings denied, and CI will run the full Linux workspace checks. No benchmark was run because this is an API convenience constructor that delegates to the existing constructor and does not change runtime execution. ## Are there any user-facing changes? Yes. This adds the public `Coercion::new_implicit_native` convenience API. It is additive and backward compatible; existing `new_implicit` callers continue to work unchanged. --------- Co-authored-by: jiangzhen <[email protected]> --- datafusion/expr-common/src/signature.rs | 43 ++++++++++++++++++++-- .../src/approx_percentile_cont.rs | 10 ++--- datafusion/functions/src/unicode/reverse.rs | 7 ++-- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/datafusion/expr-common/src/signature.rs b/datafusion/expr-common/src/signature.rs index f0010f0a05..2bde7c89a8 100644 --- a/datafusion/expr-common/src/signature.rs +++ b/datafusion/expr-common/src/signature.rs @@ -1025,17 +1025,16 @@ fn get_data_types(native_type: &NativeType) -> Vec<DataType> { /// # Examples /// /// ``` -/// use datafusion_common::types::{logical_binary, logical_string, NativeType}; +/// use datafusion_common::types::{logical_binary, logical_string}; /// use datafusion_expr_common::signature::{Coercion, TypeSignatureClass}; /// /// // Exact coercion that only accepts timestamp types /// let exact = Coercion::new_exact(TypeSignatureClass::Timestamp); /// /// // Implicit coercion that accepts string types but can coerce from binary types -/// let implicit = Coercion::new_implicit( -/// TypeSignatureClass::Native(logical_string()), +/// let implicit = Coercion::new_implicit_native( +/// logical_string(), /// vec![TypeSignatureClass::Native(logical_binary())], -/// NativeType::String, /// ); /// ``` /// @@ -1118,6 +1117,25 @@ impl Coercion { } } + /// Create a new coercion to a native logical type with implicit coercion rules. + /// + /// This is a convenience method for coercing to a specific logical type, + /// avoiding the need to separately pass the matching native default cast type. + /// + /// The native type of `desired_type` is used as the default type when coercing + /// from `allowed_source_types`. + pub fn new_implicit_native( + desired_type: LogicalTypeRef, + allowed_source_types: Vec<TypeSignatureClass>, + ) -> Self { + let default_casted_type = desired_type.native().clone(); + Self::new_implicit( + TypeSignatureClass::Native(desired_type), + allowed_source_types, + default_casted_type, + ) + } + pub fn with_encoding_preservation( mut self, encoding_preservation: EncodingPreservation, @@ -2245,6 +2263,23 @@ mod tests { assert_snapshot!(implicit_with_multiple_sources, @"Int64"); } + #[test] + fn test_new_implicit_native() { + let allowed_source_types = vec![TypeSignatureClass::Numeric]; + + assert_eq!( + Coercion::new_implicit_native( + logical_float64(), + allowed_source_types.clone(), + ), + Coercion::new_implicit( + TypeSignatureClass::Native(logical_float64()), + allowed_source_types, + NativeType::Float64, + ) + ); + } + #[test] fn test_coercion_encoding_preservation_affects_equality() { assert!(!EncodingPreservation::default().preserve_dictionary()); diff --git a/datafusion/functions-aggregate/src/approx_percentile_cont.rs b/datafusion/functions-aggregate/src/approx_percentile_cont.rs index ea8fea1b1b..4af3574d8b 100644 --- a/datafusion/functions-aggregate/src/approx_percentile_cont.rs +++ b/datafusion/functions-aggregate/src/approx_percentile_cont.rs @@ -139,10 +139,9 @@ impl ApproxPercentileCont { vec![TypeSignatureClass::Numeric], NativeType::Float64, ), - Coercion::new_implicit( - TypeSignatureClass::Native(logical_float64()), + Coercion::new_implicit_native( + logical_float64(), vec![TypeSignatureClass::Numeric], - NativeType::Float64, ), ]), // 3 args - numeric, percentile (float), number of centroid for T-Digest (integer) @@ -152,10 +151,9 @@ impl ApproxPercentileCont { vec![TypeSignatureClass::Numeric], NativeType::Float64, ), - Coercion::new_implicit( - TypeSignatureClass::Native(logical_float64()), + Coercion::new_implicit_native( + logical_float64(), vec![TypeSignatureClass::Numeric], - NativeType::Float64, ), Coercion::new_implicit( TypeSignatureClass::Integer, diff --git a/datafusion/functions/src/unicode/reverse.rs b/datafusion/functions/src/unicode/reverse.rs index 9dfc25fbdf..e42240ba66 100644 --- a/datafusion/functions/src/unicode/reverse.rs +++ b/datafusion/functions/src/unicode/reverse.rs @@ -25,7 +25,7 @@ use DataType::{LargeUtf8, Utf8, Utf8View}; use arrow::array::{Array, ArrayRef, AsArray, StringArrayType}; use arrow::datatypes::DataType; use datafusion_common::Result; -use datafusion_common::types::{NativeType, logical_string}; +use datafusion_common::types::logical_string; use datafusion_expr::{ Coercion, ColumnarValue, Documentation, EncodingPreservation, ScalarFunctionArgs, ScalarUDFImpl, Signature, TypeSignatureClass, Volatility, @@ -62,10 +62,9 @@ impl ReverseFunc { Self { signature: Signature::coercible( vec![ - Coercion::new_implicit( - TypeSignatureClass::Native(logical_string()), + Coercion::new_implicit_native( + logical_string(), vec![TypeSignatureClass::Any], - NativeType::String, ) .with_encoding_preservation(EncodingPreservation::dictionary()), ], --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
