cblp opened a new pull request, #27102: URL: https://github.com/apache/datafusion/pull/27102
## Which issue does this PR close? - Closes #18235. ## Rationale for this change `CREATE FUNCTION` drops the metadata of argument and return types, so a `FunctionFactory` cannot tell an extension type from its storage type. For example, for ```sql CREATE FUNCTION same_uuid(id UUID) RETURNS UUID RETURN $1 ``` the factory receives `FixedSizeBinary(16)` for the argument and the return type, without `ARROW:extension:name = arrow.uuid`. The placeholder `$1` in the body also has the type `FixedSizeBinary(16)`. The SQL planner already resolves these types to fields with metadata (`convert_data_type_to_field`), but `CreateFunction` stores them as `DataType`s, so the planner kept only the data type. ## What changes are included in this PR? - `OperateFunctionArg::data_type: DataType` is replaced by `OperateFunctionArg::field: FieldRef`, and `CreateFunction::return_type: Option<DataType>` by `CreateFunction::return_field: Option<FieldRef>`. This follows how `Placeholder` (#17986) and `ScalarVariable` (#18243) moved from `DataType` to `FieldRef`. `CreateFunction::try_new` takes the return type as `Option<FieldRef>`. - The SQL planner keeps the fields it resolves. The argument field is named after the argument, or `""` if it has no name. Before, the planner built the fields for the placeholders in the body from the data types, with the same names. Now it reuses the argument fields, so the placeholders keep the metadata too. - The `function_factory` example and the existing `FunctionFactory` tests read the data type from the fields. - An entry in the 56.0.0 upgrade guide. ## What is the testing strategy for this PR? `create_function_keeps_extension_types` in `datafusion/core/tests/user_defined/user_defined_scalar_functions.rs` plans `CREATE FUNCTION same_uuid(id UUID) RETURNS UUID RETURN $1` with a recording `FunctionFactory`. It checks that the argument field, the return field and the placeholder in the body all have the UUID extension type. The test fails if the planner drops the metadata. This is a Rust test rather than a sqllogictest because DataFusion has no built-in `FunctionFactory`, so what `CREATE FUNCTION` passes to it cannot be observed from SQL. The existing `CREATE FUNCTION` tests are updated for the new fields. ## Are there any user-facing changes? Yes, this is a breaking API change, so it needs the `api change` label: - `OperateFunctionArg::data_type` is renamed to `field` and is now a `FieldRef`; - `CreateFunction::return_type` is renamed to `return_field` and is now an `Option<FieldRef>`. `FunctionFactory` implementations that read these fields need to call `.data_type()` on them, or can use the fields to support extension types. The upgrade guide describes the migration. There is no change in SQL behavior without a custom `FunctionFactory`. -- 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]
