Skip to content

Conversation

@cloud-fan
Copy link
Contributor

instead of return false, throw exception when check equality between external and internal row is better.

@cloud-fan
Copy link
Contributor Author

cc @rxin

@rxin
Copy link
Contributor

rxin commented Jul 17, 2015

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

technically you want .eq instead of == i think, because == actually triggers the equality of the other function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but eq accepts AnyRef, here o is Any. I googled it, looks like x == null is OK for null check in scala.

Copy link
Contributor

Choose a reason for hiding this comment

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

alright then lgtm

@rxin
Copy link
Contributor

rxin commented Jul 17, 2015

let's see if we have any places that we mistakenly compare

@SparkQA
Copy link

SparkQA commented Jul 17, 2015

Test build #37599 timed out for PR 7460 at commit 402daa8 after a configured wait of 175m.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

finally figure out how to use eq to do null check here...

@SparkQA
Copy link

SparkQA commented Jul 17, 2015

Test build #37632 has finished for PR 7460 at commit 8a20911.

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

@rxin
Copy link
Contributor

rxin commented Jul 17, 2015

Thanks - merging this in.

@asfgit asfgit closed this in 59d24c2 Jul 17, 2015
@cloud-fan cloud-fan deleted the row-compare branch July 17, 2015 17:07
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