chore: remove deprecated ScalarUDF / ScalarUDFImpl methods past the API health policy - #24561
Open
MannXo wants to merge 1 commit into
Open
chore: remove deprecated ScalarUDF / ScalarUDFImpl methods past the API health policy#24561MannXo wants to merge 1 commit into
MannXo wants to merge 1 commit into
Conversation
The API health policy keeps a deprecated API for 6 major versions or 6 months, whichever is longer. main is at 55.0.0, so anything deprecated in 50.0.0 or earlier can be removed. Removes ScalarUDFImpl::is_nullable (45.0.0), ScalarUDF::display_name, ScalarUDF::is_nullable and ScalarUDFImpl::display_name (all 50.0.0), along with the matching AliasedScalarUDFImpl delegates and the three display_name overrides on ArrayElement, ArraySlice and GetFieldFunc. Each of those three defines its own schema_name, which is what names output columns, so query output is unchanged. Part of apache#24535
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
ScalarUDF/ScalarUDFImplmethods that are past the API health policy #24535.Rationale for this change
The API health policy deprecation guidelines keep a deprecated API for 6 major versions or 6 months, whichever is longer.
mainis at55.0.0, so anything deprecated in50.0.0or earlier can now be removed.What changes are included in this PR?
The four methods named in the issue, all in
datafusion/expr/src/udf.rs:ScalarUDFImpl::is_nullableScalarUDF::display_nameScalarUDF::is_nullableScalarUDFImpl::display_nameRemoving them takes five more sites with them, which is worth spelling out since only one of the five is in the same file.
Two are the matching delegates on
AliasedScalarUDFImpl, inudf.rs. That impl is annotated#[warn(clippy::missing_trait_methods)]and deliberately mirrors everyScalarUDFImplmethod, so the pair has to go at the same time.Three are
display_nameoverrides in other crates, onArrayElementandArraySliceindatafusion/functions-nested/src/extract.rs, and onGetFieldFuncindatafusion/functions/src/core/getfield.rs. Rust fires the deprecation lint at call sites rather than atimplblocks, so these three carried no#[expect(deprecated)]marker and produced no warning before this change. Each of them also defines its ownschema_name, which is the method that actually names output columns, so removingdisplay_namechanges no query output.ExprSchemahas no remaining use inudf.rsand comes out of the import list. That is the only added line in the diff; everything else is deletion.No
56.0.0upgrade guide note, per the issue.Are these changes tested?
Covered by existing tests. The removed code was unreachable, so there is no new behaviour to add a test for.
Run locally on
1.97.0:Extended test suite, per
AGENTS.md:68 test binaries, 10702 passed, 0 failed, 8 ignored (the 8 are pre-existing
#[ignore]tests, untouched here).Are there any user-facing changes?
Yes. This removes public Rust API, so the
api changelabel applies.Downstream implementors of
ScalarUDFImplthat overridedisplay_nameoris_nullablewill get a compile error naming the method. Callers ofScalarUDF::display_nameorScalarUDF::is_nullablewere already getting a deprecation warning and will now get a compile error.is_nullableis replaced byreturn_field_from_args;display_namehas no replacement because nothing in the codebase called it.