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

support window functions #1112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

onursatici
Copy link

Which issue does this PR close?

Closes #542 .

Rationale for this change

As stated in #542, datafusion adds SortExec and RepartitionExec based on the window clauses partition by and order by (See enforce_distribution.rs in datafusion and WindowAggExec::required_input_distribution .
From what I can see, this restriction on window functions in ballista was put way back when datafusion was in the same repo as ballista and window function support was still in its infancy. Now that datafusion enforces the right hash distribution by placing repartition nodes, I believe ballista currently already supports window functions.

I have tested partitioned, unpartitioned and sorted window expressions with ballista with this PR and compared them with datafusion, it seems to be working as intended. Parititioned window functions get repartitioned by their partition expressions and are parallelised.

Not sure what is the best way to test this given distribution handling is done by datafusion here.

What changes are included in this PR?

Allow queries with window expressions

Are there any user-facing changes?

@milenkovicm
Copy link
Contributor

It would be of great help if your could provide a test for this change @onursatici, could you ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support window function in ballista
2 participants