[SPARK-9785] [SQL] HashPartitioning compatibility should consider expression ordering #8074
Conversation
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
Contributor
Author
Contributor
|
@JoshRosen Could you add a sql test (join two DataFrame they are already partitioned on the same group of keys but different orders)? otherwise LGTM. |
JoshRosen
changed the title
|
Test build #40307 has finished for PR 8074 at commit
|
Contributor
|
LGTM. In future, it will be good to generate hashcodes for a hash partitioning in a ordering insensitive way. |
Contributor
Author
|
@davies, I don't know whether it's actually straightforward to write an end-to-end DataFrame test case which is partitioned on the same keys in different orders, although it might be achievable by joining together two group-by results. |
Contributor
Author
|
I've had a hard time contriving an end-to-end test where this bug presents a problem, but nevertheless I think that we should merge this fix. |
Contributor
|
LGTM |
|
Test build #40400 has finished for PR 8074 at commit
|
Contributor
|
I am merging it to master and branch 1.5. |
asfgit
pushed a commit
that referenced
this pull request
Aug 11, 2015
…ression ordering HashPartitioning compatibility is currently defined w.r.t the _set_ of expressions, but the ordering of those expressions matters when computing hash codes; this could lead to incorrect answers if we mistakenly avoided a shuffle based on the assumption that HashPartitionings with the same expressions in different orders will produce equivalent row hashcodes. The first commit adds a regression test which illustrates this problem. The fix for this is simple: make `HashPartitioning.compatibleWith` and `HashPartitioning.guarantees` sensitive to the expression ordering (i.e. do not perform set comparison). Author: Josh Rosen <joshrosen@databricks.com> Closes #8074 from JoshRosen/hashpartitioning-compatiblewith-fixes and squashes the following commits: b61412f [Josh Rosen] Demonstrate that I haven't cheated in my fix 0b4d7d9 [Josh Rosen] Update so that clusteringSet is only used in satisfies(). dc9c9d7 [Josh Rosen] Add failing regression test for SPARK-9785 (cherry picked from commit dfe347d) Signed-off-by: Yin Huai <yhuai@databricks.com>
Contributor
|
I think you can use to expose the problem. |
CodingCat
pushed a commit
to CodingCat/spark
that referenced
this pull request
Aug 17, 2015
…ression ordering HashPartitioning compatibility is currently defined w.r.t the _set_ of expressions, but the ordering of those expressions matters when computing hash codes; this could lead to incorrect answers if we mistakenly avoided a shuffle based on the assumption that HashPartitionings with the same expressions in different orders will produce equivalent row hashcodes. The first commit adds a regression test which illustrates this problem. The fix for this is simple: make `HashPartitioning.compatibleWith` and `HashPartitioning.guarantees` sensitive to the expression ordering (i.e. do not perform set comparison). Author: Josh Rosen <joshrosen@databricks.com> Closes apache#8074 from JoshRosen/hashpartitioning-compatiblewith-fixes and squashes the following commits: b61412f [Josh Rosen] Demonstrate that I haven't cheated in my fix 0b4d7d9 [Josh Rosen] Update so that clusteringSet is only used in satisfies(). dc9c9d7 [Josh Rosen] Add failing regression test for SPARK-9785
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
HashPartitioning compatibility is currently defined w.r.t the set of expressions, but the ordering of those expressions matters when computing hash codes; this could lead to incorrect answers if we mistakenly avoided a shuffle based on the assumption that HashPartitionings with the same expressions in different orders will produce equivalent row hashcodes. The first commit adds a regression test which illustrates this problem.
The fix for this is simple: make
HashPartitioning.compatibleWithandHashPartitioning.guaranteessensitive to the expression ordering (i.e. do not perform set comparison).