alexandreyc opened a new pull request, #20868:
URL: https://github.com/apache/datafusion/pull/20868
## Which issue does this PR close?
Closes #18643.
## Rationale for this change
See the issue.
## What changes are included in this PR?
I followed the proposal made by @2010YOUY01 in #18643.
- Added a `is_builtin() -> bool` method to `AggregateUDF/AggregateUDFImpl`
and to `WindowUDF/WindowUDFImpl`
- Updated all implementations of those traits
- Updated occurences of matching on function names to additionally check the
origin of the function
## Are these changes tested?
Not directly but I can't see a relevant way to test this. Suggestions are
welcome.
## Are there any user-facing changes?
Currently yes, but the change could be made non-breaking, see questions
below.
## Request for advices
I'm new to the codebase so feel free to challenge this PR. In particular,
I'd like to have your opinion on the following items:
1. Should we make `is_builtin` have a default implementation that returns
`false`? That would make this change non-breaking for users and slightly
simplify this PR. But in return it would be more error-prone when implementing
built-in functions.
2. Should we add the method `is_builtin` to `ScalarUDF`/`ScalarUPDImpl`? I
didn't do it because it seems there doesn't exist any scalar UDF name matching
in the codebase. It might be desirable to add it for the sake of consistency
across all kinds of UDF.
3. Should we replace `is_builtin() -> bool` by `origin() -> UDFOrigin`?
`UDFOrigin` would be something like `enum { BuiltIn, Spark, UserDefined }`.
Asking because it's not clear to me if functions in the `datafusion_spark`
crates should be considered built-in or not.
--
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]
