[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user caneGuy commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141301509
--- Diff:
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,
--- End diff --
I think the comment of `CheckCartesianProducts` should also be updated and
add this constrain.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141406147
--- Diff:
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,
--- End diff --
@gatorsmile I see, thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141405220
--- Diff:
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,
--- End diff --
Create a follow-up PR
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141404707
--- Diff:
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,
--- End diff --
Make sense. The PR is merged, what should I do now..
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141403817
--- Diff:
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,
--- End diff --
we should also add a comment here about the positioning ...
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/19362 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/19362#discussion_r141275477
--- Diff: 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") {
--- End diff --
It make make the test more concise if you just test the various join in a
single test.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19362: [SPARK-22141][SQL] Propagate empty relation befor...
GitHub user gengliangwang opened a pull request:
https://github.com/apache/spark/pull/19362
[SPARK-22141][SQL] Propagate empty relation before checking Cartesian
products
## 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 , [a#23]
+- LocalRelation , [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
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gengliangwang/spark MoveCheckCartesianProducts
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19362.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19362
commit 0f25a074821481e89a256824b40acf78a06841e5
Author: Wang Gengliang
Date: 2017-09-27T07:51:08Z
propagate empty relation before checking Cartesian products
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
