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 2017/11/27 12:48:41 UTC

[GitHub] spark pull request #19825: [SPARK-22615][SQL] Handle more cases in Propagate...

GitHub user gengliangwang opened a pull request:

    https://github.com/apache/spark/pull/19825

    [SPARK-22615][SQL] Handle more cases in PropagateEmptyRelation

    ## What changes were proposed in this pull request?
    
    Currently, in the optimize rule `PropagateEmptyRelation`, the following cases is not handled:
    1. right child is empty relation in left outer join
    2. left child in right outer join
    3. right child is empty relation in left semi join
    4. right child is empty relation in left anti join
    case #1 and #2 can be treated as **cross join** and cause exception. See the test case in JoinSuite.
    
    ## 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 SPARK-22615

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/19825.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 #19825
    
----
commit cde04c8a42a545dd3fe04044637b7e50a64053bc
Author: Wang Gengliang <lt...@gmail.com>
Date:   2017-11-27T12:44:14Z

    Handle more cases in PropagateEmptyRelation

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84217 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84217/testReport)** for PR 19825 at commit [`d6ffcdf`](https://github.com/apache/spark/commit/d6ffcdfe8b3e34c4b0eb52cde5ac0aa7c4457e1f).
     * 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 issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84284 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84284/testReport)** for PR 19825 at commit [`c359fe3`](https://github.com/apache/spark/commit/c359fe33de8425f932c0d447de358e98c0d3eb84).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84288 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84288/testReport)** for PR 19825 at commit [`189d590`](https://github.com/apache/spark/commit/189d5903f1923521239badfe82ce3835e2431bdd).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    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 #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Hi @gatorsmile , I have just fixed full outer join and add more test cases.
    It is ready for review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84288 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84288/testReport)** for PR 19825 at commit [`189d590`](https://github.com/apache/spark/commit/189d5903f1923521239badfe82ce3835e2431bdd).
     * 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 issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    LGTM


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    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 #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84288/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84217 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84217/testReport)** for PR 19825 at commit [`d6ffcdf`](https://github.com/apache/spark/commit/d6ffcdfe8b3e34c4b0eb52cde5ac0aa7c4457e1f).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84217/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84284 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84284/testReport)** for PR 19825 at commit [`c359fe3`](https://github.com/apache/spark/commit/c359fe33de8425f932c0d447de358e98c0d3eb84).
     * This patch **fails due to an unknown error code, -9**.
     * 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 issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84284/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84216/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    Thanks! Merged to master.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84216/testReport)** for PR 19825 at commit [`cde04c8`](https://github.com/apache/spark/commit/cde04c8a42a545dd3fe04044637b7e50a64053bc).
     * 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 issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    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 #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    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 pull request #19825: [SPARK-22615][SQL] Handle more cases in Propagate...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19825#discussion_r153345990
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala ---
    @@ -208,6 +208,21 @@ class JoinSuite extends QueryTest with SharedSQLContext {
         }
       }
     
    +  test("SPARK-22615: Empty relation child in left outer/right outer join should pass" +
    +    " Cartesian product check") {
    +    val x = testData2.where($"a" === 2).as("x")
    +    val y = testData2.where($"a" === 1 && !($"a" === 1)).as("y")
    +    checkAnswer(x.join(y, Seq.empty, "left_outer"),
    +      Row(2, 1, null, null) ::
    +      Row(2, 2, null, null) :: Nil
    +    )
    +
    +    checkAnswer(y.join(x, Seq.empty, "right_outer"),
    +      Row(null, null, 2, 1) ::
    +      Row(null, null, 2, 2) :: Nil
    +    )
    +  }
    --- End diff --
    
    The fix looks good to me. Instead of adding it here, to be safer, could you write a suite `joinEmptyRelation.sql` in `SQLQueryTestSuite`? Then, we can easily run all them against the other DBMS for ensuring the results are correct.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #19825: [SPARK-22615][SQL] Handle more cases in PropagateEmptyRe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19825
  
    **[Test build #84216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84216/testReport)** for PR 19825 at commit [`cde04c8`](https://github.com/apache/spark/commit/cde04c8a42a545dd3fe04044637b7e50a64053bc).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #19825: [SPARK-22615][SQL] Handle more cases in Propagate...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/19825


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org