Skip to content

Make AggregateFunction take a single argument #444

New issue

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

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

Already on GitHub? # to your account

Closed
Dandandan opened this issue May 30, 2021 · 2 comments
Closed

Make AggregateFunction take a single argument #444

Dandandan opened this issue May 30, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@Dandandan
Copy link
Contributor

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
This makes the representation of aggregate functions more correct, makes code dealing with aggregate functions more simple.

Describe the solution you'd like
Instead of Vec<Expr> use Box<Expr>

Describe alternatives you've considered
Keeping it as is.

Additional context

@jorgecarleitao
Copy link
Member

jorgecarleitao commented May 30, 2021

fyi, the goal of being Vec was to allow multi-argument aggregates. An example is the pearson correlation between two columns (which can currently be implemented via a UDAF)

@Dandandan
Copy link
Contributor Author

@jorgecarleitao Ah, yeah I think that makes sense. I'll just take care of it.

I think there is still some code (e.g. in Ballista protobuf I believe) that assumes only 1 argument, so when we are going to support that, it should be adapted.

unkloud pushed a commit to unkloud/datafusion that referenced this issue Mar 23, 2025
* add benchmarking guide

* add ASF header
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants