You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by viirya <gi...@git.apache.org> on 2018/08/27 00:29:55 UTC

[GitHub] spark pull request #22239: [SPARK-19355][SQL][Followup] Remove the child.out...

GitHub user viirya opened a pull request:

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

    [SPARK-19355][SQL][Followup] Remove the child.outputPartitioning check

    ## What changes were proposed in this pull request?
    
    This is based on the discussion https://github.com/apache/spark/pull/16677/files#r212805327.
    
    As SQL standard doesn't mandate that a nested order by followed by a limit has to respect that ordering clause, this patch removes the `child.outputPartitioning` check.
    
    ## How was this patch tested?
    
    Unit tests.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/viirya/spark-1 improve-global-limit-parallelism-followup

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

    https://github.com/apache/spark/pull/22239.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 #22239
    
----
commit b57634bea6213b67eae896489e913fac685d5a75
Author: Liang-Chi Hsieh <vi...@...>
Date:   2018-08-27T00:26:06Z

    Remove the child.outputPartitioning check.

----


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    @maropu Thanks. I just added it.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    **[Test build #95272 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95272/testReport)** for PR 22239 at commit [`b57634b`](https://github.com/apache/spark/commit/b57634bea6213b67eae896489e913fac685d5a75).
     * This patch **fails Spark unit 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 #22239: [SPARK-19355][SQL][Followup] Remove the child.outputOrde...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    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 #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2559/
    Test PASSed.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by hvanhovell <gi...@git.apache.org>.

Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    Setting `spark.sql.limit.flatGlobalLimit` to `false` works for me.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2560/
    Test PASSed.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    Sure, thank you @hvanhovell 


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by hvanhovell <gi...@git.apache.org>.

Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    @viirya did you try to run `TakeOrderedAndProjectSuite`? I am pretty sure that will fail now ;)...


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

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


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    cc @hvanhovell 


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

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


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputOrde...

Posted by cloud-fan <gi...@git.apache.org>.

Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    thanks, merging to master!


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    @hvanhovell I can set `spark.sql.limit.flatGlobalLimit` to false to match `TakeOrderedAndProjectExec` semantics at the beginning of `TakeOrderedAndProjectSuite`.  Or you prefer to add an explicit flag in `GlobalLimitExec` as you mentioned in https://github.com/apache/spark/pull/16677#discussion_r212830045? I think it may not necessarily to add the flag for now.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by hvanhovell <gi...@git.apache.org>.

Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    LGTM - Let's wait a little bit with merging to allow others to comment.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    **[Test build #95273 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95273/testReport)** for PR 22239 at commit [`67ed97d`](https://github.com/apache/spark/commit/67ed97d0ab9ba34c4fef344cbd01a2d20566b70f).


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputOrde...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    @hvanhovell ah, updated. also updated the PR description too.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    > @viirya did you try to run TakeOrderedAndProjectSuite? I am pretty sure that will fail now ;)...
    
    Not yet. Let me try.


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by hvanhovell <gi...@git.apache.org>.

Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    Shall we rename it to: **[SPARK-19355][SQL][Followup] Remove the child.outputOrdering check in global limit**?


---

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

[GitHub] spark issue #22239: [SPARK-19355][SQL][Followup] Remove the child.outputPart...

Posted by hvanhovell <gi...@git.apache.org>.

Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/22239
  
    cc @cloud-fan for a sanity check.


---

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