MannXo opened a new pull request, #24561: URL: https://github.com/apache/datafusion/pull/24561
## Which issue does this PR close? - Closes #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. `main` is at `55.0.0`, so anything deprecated in `50.0.0` or earlier can now be removed. [API health policy deprecation guidelines]: https://datafusion.apache.org/contributor-guide/api-health.html#deprecation-guidelines ## What changes are included in this PR? The four methods named in the issue, all in `datafusion/expr/src/udf.rs`: | Item | Deprecated in | | --- | --- | | `ScalarUDFImpl::is_nullable` | 45.0.0 | | `ScalarUDF::display_name` | 50.0.0 | | `ScalarUDF::is_nullable` | 50.0.0 | | `ScalarUDFImpl::display_name` | 50.0.0 | Removing 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`, in `udf.rs`. That impl is annotated `#[warn(clippy::missing_trait_methods)]` and deliberately mirrors every `ScalarUDFImpl` method, so the pair has to go at the same time. Three are `display_name` overrides in other crates, on `ArrayElement` and `ArraySlice` in `datafusion/functions-nested/src/extract.rs`, and on `GetFieldFunc` in `datafusion/functions/src/core/getfield.rs`. Rust fires the deprecation lint at call sites rather than at `impl` blocks, so these three carried no `#[expect(deprecated)]` marker and produced no warning before this change. Each of them also defines its own `schema_name`, which is the method that actually names output columns, so removing `display_name` changes no query output. `ExprSchema` has no remaining use in `udf.rs` and comes out of the import list. That is the only added line in the diff; everything else is deletion. No `56.0.0` upgrade 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`: ``` cargo fmt --all -- --check clean cargo clippy --all-targets --all-features -- -D warnings clean ./dev/rust_lint.sh clean (taplo, hawkeye, typos, prettier, rust_docs, workflow check) ``` Extended test suite, per `AGENTS.md`: ``` RUST_BACKTRACE=1 cargo test --profile ci \ --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli \ --workspace --lib --tests --bins \ --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption ``` 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 change` label applies. Downstream implementors of `ScalarUDFImpl` that override `display_name` or `is_nullable` will get a compile error naming the method. Callers of `ScalarUDF::display_name` or `ScalarUDF::is_nullable` were already getting a deprecation warning and will now get a compile error. `is_nullable` is replaced by `return_field_from_args`; `display_name` has no replacement because nothing in the codebase called it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
