[SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19362

gengliangwang · 2017-09-27T08:05:31Z

What changes were proposed in this pull request?

When inferring constraints from children, Join's condition can be simplified as None.
For example,

val testRelation = LocalRelation('a.int)
val x = testRelation.as("x")
val y = testRelation.where($"a" === 2 && !($"a" === 2)).as("y")
x.join.where($"x.a" === $"y.a")

The plan will become

Join Inner
:- LocalRelation <empty>, [a#23]
+- LocalRelation <empty>, [a#224]

And the Cartesian products check will throw exception for above plan.

Propagate empty relation before checking Cartesian products, and the issue is resolved.

How was this patch tested?

Unit test

hvanhovell · 2017-09-27T08:33:46Z

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala

@@ -200,6 +200,30 @@ class JoinSuite extends QueryTest with SharedSQLContext {
      Nil)
  }

+  test("inner join, propagate empty relation before checking Cartesian products") {


It make make the test more concise if you just test the various join types in a single test.

hvanhovell

LGTM - pending jenkins

SparkQA · 2017-09-27T10:41:52Z

Test build #82229 has finished for PR 19362 at commit 0f25a07.

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

hvanhovell · 2017-09-27T10:45:18Z

Merging to master. Thanks!

Can you open a backport if we need to port it to 2.2?

SparkQA · 2017-09-27T11:35:25Z

Test build #82230 has finished for PR 19362 at commit b721017.

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

… products When inferring constraints from children, Join's condition can be simplified as None. For example, ``` val testRelation = LocalRelation('a.int) val x = testRelation.as("x") val y = testRelation.where($"a" === 2 && !($"a" === 2)).as("y") x.join.where($"x.a" === $"y.a") ``` The plan will become ``` Join Inner :- LocalRelation <empty>, [a#23] +- LocalRelation <empty>, [a#224] ``` And the Cartesian products check will throw exception for above plan. Propagate empty relation before checking Cartesian products, and the issue is resolved. Unit test Author: Wang Gengliang <ltnwgl@gmail.com> Closes apache#19362 from gengliangwang/MoveCheckCartesianProducts.

gengliangwang · 2017-09-27T11:53:05Z

@hvanhovell Got it, I have created #19366 for the back port.

… Cartesian products Back port #19362 to branch-2.2 ## What changes were proposed in this pull request? When inferring constraints from children, Join's condition can be simplified as None. For example, ``` val testRelation = LocalRelation('a.int) val x = testRelation.as("x") val y = testRelation.where($"a" === 2 && !($"a" === 2)).as("y") x.join.where($"x.a" === $"y.a") ``` The plan will become ``` Join Inner :- LocalRelation <empty>, [a#23] +- LocalRelation <empty>, [a#224] ``` And the Cartesian products check will throw exception for above plan. Propagate empty relation before checking Cartesian products, and the issue is resolved. ## How was this patch tested? Unit test Author: Wang Gengliang <ltnwgl@gmail.com> Closes #19366 from gengliangwang/branch-2.2.

rxin · 2017-09-27T16:51:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

@@ -136,6 +134,8 @@ abstract class Optimizer(sessionCatalog: SessionCatalog)
    Batch("LocalRelation", fixedPoint,
      ConvertToLocalRelation,
      PropagateEmptyRelation) ::
+    Batch("Check Cartesian Products", Once,


we should also add a comment here about the positioning ...

Make sense. The PR is merged, what should I do now..

Create a follow-up PR

@gatorsmile I see, thanks!

caneGuy · 2017-09-27T10:15:19Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

@@ -136,6 +134,8 @@ abstract class Optimizer(sessionCatalog: SessionCatalog)
    Batch("LocalRelation", fixedPoint,
      ConvertToLocalRelation,
      PropagateEmptyRelation) ::
+    Batch("Check Cartesian Products", Once,


I think the comment of CheckCartesianProducts should also be updated and add this constrain.

## What changes were proposed in this pull request? Add comments for specifying the position of batch "Check Cartesian Products", as rxin suggested in apache#19362 . ## How was this patch tested? Unit test Author: Wang Gengliang <ltnwgl@gmail.com> Closes apache#19379 from gengliangwang/SPARK-22141-followup.

… Cartesian products Back port apache#19362 to branch-2.2 ## What changes were proposed in this pull request? When inferring constraints from children, Join's condition can be simplified as None. For example, ``` val testRelation = LocalRelation('a.int) val x = testRelation.as("x") val y = testRelation.where($"a" === 2 && !($"a" === 2)).as("y") x.join.where($"x.a" === $"y.a") ``` The plan will become ``` Join Inner :- LocalRelation <empty>, [a#23] +- LocalRelation <empty>, [a#224] ``` And the Cartesian products check will throw exception for above plan. Propagate empty relation before checking Cartesian products, and the issue is resolved. ## How was this patch tested? Unit test Author: Wang Gengliang <ltnwgl@gmail.com> Closes apache#19366 from gengliangwang/branch-2.2.

propagate empty relation before checking Cartesian products

0f25a07

hvanhovell reviewed Sep 27, 2017

View reviewed changes

test the various join types in a single test case.

b721017

hvanhovell approved these changes Sep 27, 2017

View reviewed changes

asfgit closed this in 9c5935d Sep 27, 2017

gengliangwang mentioned this pull request Sep 27, 2017

[Backport][SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19366

Closed

rxin reviewed Sep 27, 2017

View reviewed changes

caneGuy reviewed Sep 28, 2017

View reviewed changes

gengliangwang mentioned this pull request Sep 28, 2017

[SPARK-22141][Followup][SQL] Add comments for the order of batches #19379

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19362

[SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19362

gengliangwang commented Sep 27, 2017

hvanhovell Sep 27, 2017 •

edited

Loading

hvanhovell left a comment

SparkQA commented Sep 27, 2017

hvanhovell commented Sep 27, 2017

SparkQA commented Sep 27, 2017

gengliangwang commented Sep 27, 2017

rxin Sep 27, 2017

gengliangwang Sep 27, 2017

gatorsmile Sep 27, 2017

gengliangwang Sep 27, 2017

caneGuy Sep 27, 2017

[SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19362

[SPARK-22141][SQL] Propagate empty relation before checking Cartesian products #19362

Conversation

gengliangwang commented Sep 27, 2017

What changes were proposed in this pull request?

How was this patch tested?

hvanhovell Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

hvanhovell left a comment

Choose a reason for hiding this comment

SparkQA commented Sep 27, 2017

hvanhovell commented Sep 27, 2017

SparkQA commented Sep 27, 2017

gengliangwang commented Sep 27, 2017

rxin Sep 27, 2017

Choose a reason for hiding this comment

gengliangwang Sep 27, 2017

Choose a reason for hiding this comment

gatorsmile Sep 27, 2017

Choose a reason for hiding this comment

gengliangwang Sep 27, 2017

Choose a reason for hiding this comment

caneGuy Sep 27, 2017

Choose a reason for hiding this comment

hvanhovell Sep 27, 2017 •

edited

Loading