You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gengliangwang <gi...@git.apache.org> on 2018/01/16 17:09:43 UTC
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
GitHub user gengliangwang opened a pull request:
https://github.com/apache/spark/pull/20278
[SPARK-23079][SQL]Fix query constraints propagation with aliases
## What changes were proposed in this pull request?
Previously, PR #19201 fix the problem of non-converging constraints.
After that PR #19149 improve the loop and constraints is inferred only once.
So the problem of non-converging constraints is gone.
However, the case below will fail.
```
spark.range(5).write.saveAsTable("t")
val t = spark.read.table("t")
val left = t.withColumn("xid", $"id" + lit(1)).as("x")
val right = t.withColumnRenamed("id", "xid").as("y")
val df = left.join(right, "xid").filter("id = 3").toDF()
checkAnswer(df, Row(4, 3))
```
Because `aliasMap` replace all the aliased child. See the test case in PR for details.
This PR is to fix this bug by removing useless code for preventing non-converging constraints.
It can be also fixed with #20270, but this is much simpler and clean up the code.
## 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 FixConstraintSimple
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20278.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 #20278
----
commit c02d9b4bdccafcdf4008dcdd4c2ad9509c9acd96
Author: Wang Gengliang <lt...@...>
Date: 2018-01-16T17:05:28Z
Simple fix
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20278
**[Test build #86216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86216/testReport)** for PR 20278 at commit [`c02d9b4`](https://github.com/apache/spark/commit/c02d9b4bdccafcdf4008dcdd4c2ad9509c9acd96).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20278
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86181/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20278
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20278
**[Test build #86181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86181/testReport)** for PR 20278 at commit [`c02d9b4`](https://github.com/apache/spark/commit/c02d9b4bdccafcdf4008dcdd4c2ad9509c9acd96).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/20278#discussion_r161985424
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala ---
@@ -34,6 +34,7 @@ class InferFiltersFromConstraintsSuite extends PlanTest {
PushDownPredicate,
InferFiltersFromConstraints,
CombineFilters,
+ SimplifyBinaryComparison,
--- End diff --
It is just different from the expected result because of this line:
https://github.com/gengliangwang/spark/blob/c02d9b4bdccafcdf4008dcdd4c2ad9509c9acd96/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala#L258
These predicates can be simplified by `SimplifyBinaryComparison`. So no need to modified the expected test results .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/20278#discussion_r161984758
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala ---
@@ -160,64 +161,6 @@ class InferFiltersFromConstraintsSuite extends PlanTest {
comparePlans(optimized, correctAnswer)
}
- test("inner join with alias: don't generate constraints for recursive functions") {
--- End diff --
Yes
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20278#discussion_r161981192
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala ---
@@ -34,6 +34,7 @@ class InferFiltersFromConstraintsSuite extends PlanTest {
PushDownPredicate,
InferFiltersFromConstraints,
CombineFilters,
+ SimplifyBinaryComparison,
--- End diff --
Without this rule, the test cases will fail?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20278#discussion_r161981051
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala ---
@@ -160,64 +161,6 @@ class InferFiltersFromConstraintsSuite extends PlanTest {
comparePlans(optimized, correctAnswer)
}
- test("inner join with alias: don't generate constraints for recursive functions") {
--- End diff --
These test cases are invalid after the code changes, right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20278
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86216/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20278
**[Test build #86216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86216/testReport)** for PR 20278 at commit [`c02d9b4`](https://github.com/apache/spark/commit/c02d9b4bdccafcdf4008dcdd4c2ad9509c9acd96).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20278: [SPARK-23079][SQL]Fix query constraints propagati...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20278
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20278
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/20278
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20278: [SPARK-23079][SQL]Fix query constraints propagation with...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20278
LGTM, merging to master/2.3!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org