Skip to content

Fix LocalExecutionPlan for IndexJoin when spill enabled#15773

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:index-join-spill
Mar 5, 2021
Merged

Fix LocalExecutionPlan for IndexJoin when spill enabled#15773
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:index-join-spill

Conversation

@rschlussel
Copy link
Copy Markdown
Contributor

Index join does not support spill, so we don't ensure that we add
exchanges with fixed distribution before an index join. However, we
were still checking for fixed distribution in the LocalExecutionPlanner.
This removes the check for the number of operators since that is only
needed for spill

Test plan - Added unit test

== RELEASE NOTES ==

General Changes
* Fix an error executing index joins when join spilling is enabled

Index join does not support spill, so we don't ensure that we add
exchanges with fixed distribution before an index join.  However, we
were still checking for fixed distribution in the LocalExecutionPlanner.
This removes the check for the number of operators since that is only
needed for spill
@rschlussel rschlussel requested review from a team and sachdevs March 4, 2021 21:57
Copy link
Copy Markdown
Contributor

@sachdevs sachdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than seemingly unrelated maven fail, LGTM

@rschlussel
Copy link
Copy Markdown
Contributor Author

Other than seemingly unrelated maven fail, LGTM

Yeah, just restarted the workflow. annoyingly for github actions you can't restart only the failed jobs, and you have to wait for the whole workflow to complete before you can restart.

@rschlussel rschlussel merged commit 1b72d26 into prestodb:master Mar 5, 2021
@arhimondr arhimondr mentioned this pull request Mar 12, 2021
13 tasks
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.

3 participants