Skip to content

feat: add implicit native coercion constructor - #24282

Open
Jamison929611 wants to merge 1 commit into
apache:mainfrom
Jamison929611:feat/implicit-native-coercion
Open

feat: add implicit native coercion constructor#24282
Jamison929611 wants to merge 1 commit into
apache:mainfrom
Jamison929611:feat/implicit-native-coercion

Conversation

@Jamison929611

Copy link
Copy Markdown

Which issue does this PR close?

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.

@github-actions github-actions Bot added logical-expr Logical plan and expressions functions Changes to functions implementation labels Aug 12, 2026
@Jamison929611

Copy link
Copy Markdown
Author

The substantive GitHub Actions workflows are currently marked action_required pending maintainer approval for this fork contributor. When convenient, could a maintainer approve the workflow runs? I'll monitor and address any resulting failures or review feedback.

@alamb

alamb commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

FYI @Jefffrey you may be interested in this PR

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.28%. Comparing base (c08832d) to head (524966a).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24282      +/-   ##
==========================================
- Coverage   81.29%   81.28%   -0.01%     
==========================================
  Files        1110     1110              
  Lines      385205   385223      +18     
  Branches   385205   385223      +18     
==========================================
+ Hits       313145   313147       +2     
- Misses      53580    53593      +13     
- Partials    18480    18483       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coercion::new_implicit is confusing when desired type is TypeSignatureClass::Native

4 participants