Skip to content

[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity for self-joins. #5919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity for self-joins. #5919

wants to merge 4 commits into from

Conversation

rxin
Copy link
Contributor

@rxin rxin commented May 5, 2015

See the comment in join function for more information.

damiencarol
damiencarol reviewed May 5, 2015
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
Copy link

Choose a reason for hiding this comment

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

iff => if ???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iff == if and only if

Copy link

Choose a reason for hiding this comment

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

"if and only if" should be much more specific in the method comment IMHO.
iff can be confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

iff is a pretty standard shortening and is used throughout the spark code base already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but I disagree. We use it iff a lot in SQL / algebra, and if you Google, the first thing that comes up is "if and only if". Plus this is an internal comment.

edit: This was in response to @damiencarol's comment, not @marmbrus'.

@SparkQA
Copy link

SparkQA commented May 5, 2015

Test build #31901 has finished for PR 5919 at commit 7233a86.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor Author

rxin commented May 5, 2015

Jenkins, retest this please.

@rxin rxin closed this May 5, 2015
@rxin rxin reopened this May 5, 2015
@SparkQA
Copy link

SparkQA commented May 5, 2015

Test build #31900 has finished for PR 5919 at commit 6be2b4d.

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

@SparkQA
Copy link

SparkQA commented May 5, 2015

Test build #31903 has finished for PR 5919 at commit e2fb0da.

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

@SparkQA
Copy link

SparkQA commented May 6, 2015

Test build #31915 has finished for PR 5919 at commit e2fb0da.

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

@marmbrus
Copy link
Contributor

marmbrus commented May 6, 2015

Thanks! Merged to master.

@asfgit asfgit closed this in 1fd31ba May 6, 2015
@rxin rxin deleted the self-join-resolve branch May 6, 2015 02:02
asfgit pushed a commit that referenced this pull request May 6, 2015
@rxin @marmbrus
[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity f…
e61083c 
…or self-joins.

See the comment in join function for more information.

Author: Reynold Xin <[email protected]>

Closes #5919 from rxin/self-join-resolve and squashes the following commits:

e2fb0da [Reynold Xin] Updated SQLConf comment.
7233a86 [Reynold Xin] Updated comment.
6be2b4d [Reynold Xin] Removed println
9f6b72f [Reynold Xin] [SPARK-6231][SQL/DF] Automatically resolve ambiguity in join condition for self-joins.
@marmbrus
Copy link
Contributor

marmbrus commented May 6, 2015

... and branch 1.4

jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
@rxin @jeanlyn
[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity f…
2de4750 
…or self-joins.

See the comment in join function for more information.

Author: Reynold Xin <[email protected]>

Closes apache#5919 from rxin/self-join-resolve and squashes the following commits:

e2fb0da [Reynold Xin] Updated SQLConf comment.
7233a86 [Reynold Xin] Updated comment.
6be2b4d [Reynold Xin] Removed println
9f6b72f [Reynold Xin] [SPARK-6231][SQL/DF] Automatically resolve ambiguity in join condition for self-joins.
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@rxin @jeanlyn
[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity f…
3582d85 
…or self-joins.

See the comment in join function for more information.

Author: Reynold Xin <[email protected]>

Closes apache#5919 from rxin/self-join-resolve and squashes the following commits:

e2fb0da [Reynold Xin] Updated SQLConf comment.
7233a86 [Reynold Xin] Updated comment.
6be2b4d [Reynold Xin] Removed println
9f6b72f [Reynold Xin] [SPARK-6231][SQL/DF] Automatically resolve ambiguity in join condition for self-joins.
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@rxin @nemccarthy
[SPARK-6231][SQL/DF] Automatically resolve join condition ambiguity f…
3ec83f2 
…or self-joins.

See the comment in join function for more information.

Author: Reynold Xin <[email protected]>

Closes apache#5919 from rxin/self-join-resolve and squashes the following commits:

e2fb0da [Reynold Xin] Updated SQLConf comment.
7233a86 [Reynold Xin] Updated comment.
6be2b4d [Reynold Xin] Removed println
9f6b72f [Reynold Xin] [SPARK-6231][SQL/DF] Automatically resolve ambiguity in join condition for self-joins.
@gatorsmile
Copy link
Member

@rxin @marmbrus This fix is unable to resolve the condition ambiguity for nested self join. I also found the self joins could generate incorrect results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rxin @SparkQA @marmbrus @gatorsmile @damiencarol