fix: Check maintains_input_order in EnforceDistribution#35
Open
fix: Check maintains_input_order in EnforceDistribution#35
maintains_input_order in EnforceDistribution#35Conversation
…d does not assert that it maintains its input order. If it does, we should not replace order preserving variants
xudong963
approved these changes
Mar 23, 2026
zhuqi-lucas
approved these changes
Mar 23, 2026
Collaborator
zhuqi-lucas
left a comment
There was a problem hiding this comment.
LGTM, don't forget to cherry-pick to branch-52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In the event that there is no ordering requirement from the parent, and the required distribution is a single partition, we should make sure to check that input order is not maintained before replacing nodes with their order preserving variants.
This is already done when the parent's distribution is unspecified -- we should do the same when it's single/hash partitioned.
Context
This was discovered when debugging a seemingly sub-optimal plan post remote execution optimization (certain details have been omitted):
Each child of
RemoteExechas a declared input ordering...which the top-level SPM leverages. However, afterEnforceDistributionruns,CoalescePartitionsExecis added underRemoteExec, rather than SPM. SinceRemoteExecmaintains its input ordering,EnforceDistributionshould useSortPreservingMergeExechere instead. TheCoalescePartitionsExecends up producing a suboptimal plan that requires aSortExec, which a subsequent run ofEnforceSortingadds.