Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enabling AQE on [databricks] #6461
Enabling AQE on [databricks] #6461
Changes from 26 commits
8d8b121
b3b516c
c45cbe0
5b3f954
40e7044
d0c5ccb
acaa586
e203e9c
91dc00a
1ce22b2
c3ebbbb
4a1f591
a64a484
1682f87
da750f0
b9e2bad
d65c307
cdb32c6
4922fb4
12fe02c
29da0c1
541d91a
6e93d9c
afbdf2b
41adfa9
c470b72
3bf9f21
819d54d
bf10236
9c4288b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mythrocks mind taking a look at this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay.
I'm not particularly familiar with the logic behind
isPreNeeded
, etc. But on discussion with @NVnavkumar, one wonders if we should check whyisPreNeeded
is turning upfalse
on Databricks, with AQE turned on. We might adjust howisPreNeeded
is calculated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So,
isPreNeeded
isfalse
on Databricks, but it's a bit of a red herring in this case. We actually need to the window function children in the GpuWindowExec itself, it looks like the extra GpuProject does not help in this case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some exploration, I have determined that the bug this fixes is not AQE-specific and I'm not 100% confident that this fix is currently the right approach. I have reverted this fix and filed a new issue #6531 to track the bug here.