Skip to content

Conversation

@MasterDDT
Copy link
Contributor

@MasterDDT MasterDDT commented Jul 7, 2016

What changes were proposed in this pull request?

EnsureRequirements compares the required and given sort ordering, but uses Scala equals instead of a semantic equals, so column capitalization isn't considered, and also fails for a cached table. This results in a SortMergeJoin of a cached already-sorted table to add an extra sort.

Using semanticEquals to do the compare instead of scala equals on the two Seq[SortOrder]

How was this patch tested?

Added 3 tests, the last 2 tests break without the fix.

@MasterDDT
Copy link
Contributor Author

MasterDDT commented Jul 7, 2016

cc @JoshRosen @rxin

I think an alternative fix here is that Expression could override equals and use semanticEquals, that would be a bigger change but I think would work. Also I noticed the EquivalentExpressions class might be useful, but seemed like it was only for codegen.

@hvanhovell
Copy link
Contributor

Ok to test

@SparkQA
Copy link

SparkQA commented Jul 7, 2016

Test build #3168 has finished for PR 14092 at commit b4b02bf.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 7, 2016

Test build #3169 has finished for PR 14092 at commit b4b02bf.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MasterDDT
Copy link
Contributor Author

cc @hvanhovell

@hvanhovell
Copy link
Contributor

@MasterDDT I am sorry that I didn't get back to you sooner. We have recently merged PR #14841 which implements similar functionality. Cloud you close this PR? Thank you for working on this!

@MasterDDT
Copy link
Contributor Author

Closing this

@MasterDDT MasterDDT closed this Aug 30, 2016
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