Use return_field_from_args in information schema and date_trunc#20079
Use return_field_from_args in information schema and date_trunc#20079Jefffrey merged 4 commits intoapache:mainfrom
Conversation
| .into_iter() | ||
| .map(|arg_types| { | ||
| // only handle the function which implemented [`ScalarUDFImpl::return_type`] method | ||
| let arg_fields: Vec<FieldRef> = arg_types |
There was a problem hiding this comment.
This is interesting that we omitted window functions before 🤔
And somehow this fix doesn't affect any existing tests?
There was a problem hiding this comment.
Good morning!
If i'm allowed, this could use a dedicated test/tests, at the same time i wanted to keep It lean for reviewers.
If any follow up Is needed, please consider me for it
There was a problem hiding this comment.
I think this PR is fine to merge as is, but I wouldn't be opposed to adding some SLT tests for that case
| } | ||
|
|
||
| // keep return_type implementation for information schema generation | ||
| fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> { |
There was a problem hiding this comment.
Please update https://github.com/AndreaBozzo/datafusion/blob/e8d78e81765137be6fd7258e84cdcd62fdddd907/datafusion/functions/benches/date_trunc.rs#L61 to use return_field_from_args
There was a problem hiding this comment.
thanks this should avoid internal_error, good catch
…schema This test verifies that information_schema.routines correctly reports return types for window functions (UDWFs). Before PR apache#20079, window functions would show NULL for data_type. This test ensures the fix works correctly and prevents regression. Tests cover common window functions: - row_number, rank, dense_rank (return Int64) - lag, lead (return type depends on input, shown as Null) The tests validate that: 1. Window functions now have proper data_type values (not NULL) 2. function_type correctly identifies them as 'WINDOW' 3. Return types match the actual behavior of each function Closes apache#20090
|
Thanks @AndreaBozzo & @martin-g |
Window functions with no arguments (nullary) were showing NULL for their data_type in information_schema.routines instead of their actual return types. PR apache#20079 fixed this for window functions WITH arguments, but missed the nullary case. This commit fixes the nullary case by: - Checking if the signature is TypeSignature::Nullary - Calling WindowUDF::field() with empty args for nullary functions - Updating test expectations to match actual return types (UInt64 instead of Int64, and NULL instead of Null) - Adding expected parameter entry for rank function Now nullary window functions correctly report: - row_number: UInt64 - rank: UInt64 - dense_rank: UInt64 Co-Authored-By: Claude (claude-sonnet-4.5) <noreply@anthropic.com>
…he#20079) ## Which issue does this PR close? - Closes apache#19870. ## Rationale for this change Some UDFs/UDAFs implement `return_field_from_args` / `return_field` instead of `return_type`. The information schema was calling `return_type` directly, which fails for those functions. The default implementation of `return_field_from_args` already delegates to `return_type`, so switching to the newer API works for all functions. ## What changes are included in this PR? - **`information_schema.rs`**: `get_udf_args_and_return_types` now calls `return_field_from_args` instead of `return_type`; `get_udaf_args_and_return_types` now calls `return_field` instead of `return_type`. Removed stale comments referencing the old API. - **`date_trunc.rs`**: `return_type` now returns `internal_err`, and `return_field_from_args` is self-contained (no longer delegates to `return_type`), following the same pattern as other UDFs like `named_struct` and `map_from_arrays` (ref: apache#19275). ## Are these changes tested? Covered by existing information_schema sqllogictests and `datafusion-functions` unit tests. ## Are there any user-facing changes? No.
Which issue does this PR close?
return_field_from_args#19870.Rationale for this change
Some UDFs/UDAFs implement
return_field_from_args/return_fieldinstead ofreturn_type. The information schema was callingreturn_typedirectly, which fails for those functions. The default implementation ofreturn_field_from_argsalready delegates toreturn_type, so switching to the newer API works for all functions.What changes are included in this PR?
information_schema.rs:get_udf_args_and_return_typesnow callsreturn_field_from_argsinstead ofreturn_type;get_udaf_args_and_return_typesnow callsreturn_fieldinstead ofreturn_type. Removed stale comments referencing the old API.date_trunc.rs:return_typenow returnsinternal_err, andreturn_field_from_argsis self-contained (no longer delegates toreturn_type), following the same pattern as other UDFs likenamed_structandmap_from_arrays(ref: fix: derive custom nullability for sparkmap_from_arrays#19275).Are these changes tested?
Covered by existing information_schema sqllogictests and
datafusion-functionsunit tests.Are there any user-facing changes?
No.