Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify Expr::AggregateFunction and Expr::AggregateUDF #8346

Closed
alamb opened this issue Nov 28, 2023 · 2 comments · Fixed by #8358
Closed

Unify Expr::AggregateFunction and Expr::AggregateUDF #8346

alamb opened this issue Nov 28, 2023 · 2 comments · Fixed by #8358
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Nov 28, 2023

Is your feature request related to a problem or challenge?

Part of #8045 we are working
to consolidate the function implementation in DataFusion to make it easier to
manage DataFusion's built in list of functions as well as ensure user defined
and built in functions have the same feature sets.

I think it is also important to keep the Expr representation consistent for
Aggregate and Window functions to make DataFusion easier to work with, as well
as to permit eventually applying the same unification of built in and user
defined functions to aggregates and window functions.

Describe the solution you'd like

I would like to unify Expr::AggregateFunction and Expr::AggregateUDF following the pattern in #8258 @2010YOUY01:

  1. Remove Expr::AggregateUDF
  2. Add an AggregateFunctionDefinition that mirrors ScalarFunctionDefinition
  3. Update Expr::AggregateFunction so that it is used for both built in and user defined functions
  4. Update the code to use these new APIs

Describe alternatives you've considered

No response

Additional context

I think this is a pretty good first issue because we have an existing pattern in #8258

@alamb alamb added enhancement New feature or request good first issue Good for newcomers labels Nov 28, 2023
@Weijun-H
Copy link
Member

I will take this ticket

@alamb
Copy link
Contributor Author

alamb commented Nov 29, 2023

Thank you @Weijun-H

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants