You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ptkool <gi...@git.apache.org> on 2017/05/08 10:35:48 UTC

[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

GitHub user ptkool opened a pull request:

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

    [SPARK-20636] Add new optimization rule to flip adjacent Window expressions.

    ## What changes were proposed in this pull request?
    
    Add new optimization rule to eliminate unnecessary shuffling by flipping adjacent Window expressions.
    
    ## How was this patch tested?
    
    Tested with unit tests, integration tests, and manual tests.

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

    $ git pull https://github.com/ptkool/spark adjacent_window_optimization

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

    https://github.com/apache/spark/pull/17899.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 #17899
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116693306
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps1.length < ps2.length && ps2.containsSlice(ps1) =>
    +      Project(w1.output, Window(we2, ps2, os2, Window(we1, ps1, os1, grandChild)))
    --- End diff --
    
    This probably warrants a follow-up that tries to move projections that are wedged in between two window clauses. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r117240492
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala ---
    @@ -423,4 +423,25 @@ class DataFrameWindowFunctionsSuite extends QueryTest with SharedSQLContext {
           df.select(selectList: _*).where($"value" < 2),
           Seq(Row(3, "1", null, 3.0, 4.0, 3.0), Row(5, "1", false, 4.0, 5.0, 5.0)))
       }
    +
    +  test("window functions in multiple selects") {
    --- End diff --
    
    I'll remove it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #95802 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95802/testReport)** for PR 17899 at commit [`4328831`](https://github.com/apache/spark/commit/432883117b8d067a821422d68fcfc3a03ca36527).
     * 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 #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83194 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83194/testReport)** for PR 17899 at commit [`e9f6928`](https://github.com/apache/spark/commit/e9f6928bb60e7c4de25324e5572f105a30d16cd5).
     * 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 #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #86355 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86355/testReport)** for PR 17899 at commit [`72a4b3a`](https://github.com/apache/spark/commit/72a4b3a52091b8a14f44d703d9827a67274b2c0b).


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r115370303
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Flip Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, flip them.
    + */
    +object FlipWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps2.containsSlice(ps1) =>
    --- End diff --
    
    You are also changing the order of the columns. You will need to add a projection on top to be sure.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76690 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76690/testReport)** for PR 17899 at commit [`1ab81ca`](https://github.com/apache/spark/commit/1ab81ca36ee967d6a01561a0e798438892b5baa0).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83189/
    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 #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r124966795
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -610,6 +611,25 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is compatible with the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  private def compatibleParititions(ps1 : Seq[Expression], ps2: Seq[Expression]): Boolean = {
    +    ps1.length < ps2.length && ps2.take(ps1.length).permutations.exists(ps1.zip(_).forall {
    +      case (l, r) => l.semanticEquals(r)
    +    })
    +  }
    +
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    --- End diff --
    
    The expressions in both `we1` and `we2` must be deterministic. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r117238414
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps1.length < ps2.length && ps2.containsSlice(ps1) =>
    --- End diff --
    
    Agreed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r147546887
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -610,6 +611,25 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is compatible with the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  private def compatibleParititions(ps1 : Seq[Expression], ps2: Seq[Expression]): Boolean = {
    +    ps1.length < ps2.length && ps2.take(ps1.length).permutations.exists(ps1.zip(_).forall {
    +      case (l, r) => l.semanticEquals(r)
    +    })
    +  }
    +
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    --- End diff --
    
    Just to ensure the results are still the same with and without the rule. 


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #95753 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95753/testReport)** for PR 17899 at commit [`94e8115`](https://github.com/apache/spark/commit/94e8115d64db4942012869df129d23eda3faa06b).


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76970 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76970/testReport)** for PR 17899 at commit [`1ab81ca`](https://github.com/apache/spark/commit/1ab81ca36ee967d6a01561a0e798438892b5baa0).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116718934
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala ---
    @@ -423,4 +423,25 @@ class DataFrameWindowFunctionsSuite extends QueryTest with SharedSQLContext {
           df.select(selectList: _*).where($"value" < 2),
           Seq(Row(3, "1", null, 3.0, 4.0, 3.0), Row(5, "1", false, 4.0, 5.0, 5.0)))
       }
    +
    +  test("window functions in multiple selects") {
    --- End diff --
    
    Why this test? It does not really add anything new.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76678 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76678/testReport)** for PR 17899 at commit [`300edf0`](https://github.com/apache/spark/commit/300edf0a34aaea3324508ff1c6880587ad4aa1f4).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #86355 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86355/testReport)** for PR 17899 at commit [`72a4b3a`](https://github.com/apache/spark/commit/72a4b3a52091b8a14f44d703d9827a67274b2c0b).
     * 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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #95753 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95753/testReport)** for PR 17899 at commit [`94e8115`](https://github.com/apache/spark/commit/94e8115d64db4942012869df129d23eda3faa06b).
     * 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 #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r238950241
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -734,6 +734,28 @@ object CollapseWindow extends Rule[LogicalPlan] {
       }
     }
     
    +/**
    + * Transpose Adjacent Window Expressions.
    --- End diff --
    
    why is this rule useful?


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83189 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83189/testReport)** for PR 17899 at commit [`54d88fa`](https://github.com/apache/spark/commit/54d88fabf2076dbe17f22a72526ad7879a4c8eb0).
     * This patch **fails Scala style 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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83191 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83191/testReport)** for PR 17899 at commit [`82d7390`](https://github.com/apache/spark/commit/82d7390e284b9324c75f4a0755f91fb3127f4d7f).
     * This patch **fails Scala style 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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    @ptkool Can you address the conflicts? We will review it. 


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r125013283
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -610,6 +611,25 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is compatible with the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  private def compatibleParititions(ps1 : Seq[Expression], ps2: Seq[Expression]): Boolean = {
    +    ps1.length < ps2.length && ps2.take(ps1.length).permutations.exists(ps1.zip(_).forall {
    +      case (l, r) => l.semanticEquals(r)
    +    })
    +  }
    +
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    --- End diff --
    
    Why? This seems overly restrictive to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r115346396
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Flip Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, flip them.
    + */
    +object FlipWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps2.containsSlice(ps1) =>
    --- End diff --
    
    This condition might not be enough. `we1` might depend on the outputs of `we2`, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r215843736
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -726,14 +726,36 @@ object CollapseRepartition extends Rule[LogicalPlan] {
     object CollapseWindow extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
         case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    -        if ps1 == ps2 && os1 == os2 && w1.references.intersect(w2.windowOutputSet).isEmpty &&
    --- End diff --
    
    I wouldn't include style changes


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76679 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76679/testReport)** for PR 17899 at commit [`eb2def2`](https://github.com/apache/spark/commit/eb2def2561bf67841e9f8a48c20299d2cf0247f6).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116696350
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps1.length < ps2.length && ps2.containsSlice(ps1) =>
    --- End diff --
    
    You need to check that the windows are independent, e.g.: `w1.references.intersect(w2.windowOutputSet).isEmpty`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83191 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83191/testReport)** for PR 17899 at commit [`82d7390`](https://github.com/apache/spark/commit/82d7390e284b9324c75f4a0755f91fb3127f4d7f).


---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r117240910
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/TransposeWindowSuite.scala ---
    @@ -0,0 +1,101 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.dsl.expressions._
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.expressions.{RowFrame, SpecifiedWindowFrame, UnboundedFollowing, UnboundedPreceding, UnspecifiedFrame}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
    +import org.apache.spark.sql.catalyst.rules.RuleExecutor
    +
    +
    +class TransposeWindowSuite extends PlanTest {
    +  object Optimize extends RuleExecutor[LogicalPlan] {
    +    val batches =
    +      Batch("CollapseProject", FixedPoint(100), CollapseProject, RemoveRedundantProject) ::
    +      Batch("FlipWindow", Once, CollapseWindow, TransposeWindow) :: Nil
    +  }
    +
    +  val testRelation = LocalRelation('a.string, 'b.string, 'c.int, 'd.string)
    +
    +  val a = testRelation.output(0)
    +  val b = testRelation.output(1)
    +  val c = testRelation.output(2)
    +  val d = testRelation.output(3)
    +
    +  val partitionSpec1 = Seq(a)
    +  val partitionSpec2 = Seq(a, b)
    +  val partitionSpec3 = Seq(d)
    +
    +  val orderSpec1 = Seq(d.asc)
    +  val orderSpec2 = Seq(d.desc)
    +
    +  test("flip two adjacent windows with compatible partitions in multiple selects") {
    +    val wexpr1 = windowExpr(sum('c), windowSpec(partitionSpec2, Seq.empty, UnspecifiedFrame))
    +    val wexpr2 = windowExpr(sum('c), windowSpec(partitionSpec1, Seq.empty, UnspecifiedFrame))
    +
    +    val query = testRelation
    +      .select('a, 'b, 'c, wexpr1.as('sum_a_2))
    +      .select('a, 'b, 'c, 'sum_a_2, wexpr2.as('sum_a_1))
    +
    +    val optimized = Optimize.execute(query.analyze)
    +
    +    val query2 = testRelation
    +      .select('a, 'b, 'c)
    +      .select('a, 'b, 'c, wexpr2.as('sum_a_1))
    +      .select('a, 'b, 'c, wexpr1.as('sum_a_2), 'sum_a_1)
    +      .select('a, 'b, 'c, 'sum_a_2, 'sum_a_1)
    +
    +    val correctAnswer = Optimize.execute(query2.analyze)
    +
    +    comparePlans(optimized, correctAnswer)
    +  }
    +
    +  test("flip two adjacent windows with compatible partitions") {
    +    val query = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec2, orderSpec2)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, orderSpec1)
    +
    +    val analyzed = query.analyze
    +    val optimized = Optimize.execute(analyzed)
    +
    +    val correctAnswer = testRelation
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, orderSpec1)
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec2, orderSpec2)
    +      .select('a, 'b, 'c, 'd, 'sum_a_2, 'sum_a_1)
    +
    +    comparePlans(optimized, correctAnswer.analyze)
    +  }
    +
    +  test("don't flip two adjacent windows with incompatible partitions") {
    +    val query = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec3, Seq.empty)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, Seq.empty)
    +
    +    val analyzed = query.analyze
    +    val optimized = Optimize.execute(analyzed)
    +
    +    val correctAnswer = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec3, Seq.empty)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, Seq.empty)
    +
    +    comparePlans(optimized, correctAnswer.analyze)
    --- End diff --
    
    Yes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76690 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76690/testReport)** for PR 17899 at commit [`1ab81ca`](https://github.com/apache/spark/commit/1ab81ca36ee967d6a01561a0e798438892b5baa0).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76678 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76678/testReport)** for PR 17899 at commit [`300edf0`](https://github.com/apache/spark/commit/300edf0a34aaea3324508ff1c6880587ad4aa1f4).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83189 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83189/testReport)** for PR 17899 at commit [`54d88fa`](https://github.com/apache/spark/commit/54d88fabf2076dbe17f22a72526ad7879a4c8eb0).


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76970 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76970/testReport)** for PR 17899 at commit [`1ab81ca`](https://github.com/apache/spark/commit/1ab81ca36ee967d6a01561a0e798438892b5baa0).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 pull request #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r124966455
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -610,6 +611,25 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is compatible with the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  private def compatibleParititions(ps1 : Seq[Expression], ps2: Seq[Expression]): Boolean = {
    +    ps1.length < ps2.length && ps2.take(ps1.length).permutations.exists(ps1.zip(_).forall {
    +      case (l, r) => l.semanticEquals(r)
    +    })
    +  }
    +
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if w1.references.intersect(w2.windowOutputSet).isEmpty && compatibleParititions(ps1, ps2) =>
    --- End diff --
    
    No test case covers the condition `w1.references.intersect(w2.windowOutputSet).isEmpty` 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #78973 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78973/testReport)** for PR 17899 at commit [`e2f24c2`](https://github.com/apache/spark/commit/e2f24c2abdddf1046bd7e4fd4631972aa6f7792d).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76680 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76680/testReport)** for PR 17899 at commit [`f6a4e47`](https://github.com/apache/spark/commit/f6a4e47e41034622bcfbb0dcc3cdd5400af3c33b).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #95802 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95802/testReport)** for PR 17899 at commit [`4328831`](https://github.com/apache/spark/commit/432883117b8d067a821422d68fcfc3a03ca36527).


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #83193 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83193/testReport)** for PR 17899 at commit [`f840c69`](https://github.com/apache/spark/commit/f840c69231025b53d057ec7e1032750abfa856b8).
     * This patch **fails Scala style 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 #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    @hvanhovell @gatorsmile Can you have another look at this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116719332
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/TransposeWindowSuite.scala ---
    @@ -0,0 +1,101 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.dsl.expressions._
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.expressions.{RowFrame, SpecifiedWindowFrame, UnboundedFollowing, UnboundedPreceding, UnspecifiedFrame}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
    +import org.apache.spark.sql.catalyst.rules.RuleExecutor
    +
    +
    +class TransposeWindowSuite extends PlanTest {
    +  object Optimize extends RuleExecutor[LogicalPlan] {
    +    val batches =
    +      Batch("CollapseProject", FixedPoint(100), CollapseProject, RemoveRedundantProject) ::
    +      Batch("FlipWindow", Once, CollapseWindow, TransposeWindow) :: Nil
    +  }
    +
    +  val testRelation = LocalRelation('a.string, 'b.string, 'c.int, 'd.string)
    +
    +  val a = testRelation.output(0)
    +  val b = testRelation.output(1)
    +  val c = testRelation.output(2)
    +  val d = testRelation.output(3)
    +
    +  val partitionSpec1 = Seq(a)
    +  val partitionSpec2 = Seq(a, b)
    +  val partitionSpec3 = Seq(d)
    +
    +  val orderSpec1 = Seq(d.asc)
    +  val orderSpec2 = Seq(d.desc)
    +
    +  test("flip two adjacent windows with compatible partitions in multiple selects") {
    --- End diff --
    
    I am not entirely sure if we need this test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    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 #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83191/
    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 #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r117241321
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/TransposeWindowSuite.scala ---
    @@ -0,0 +1,101 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.dsl.expressions._
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.expressions.{RowFrame, SpecifiedWindowFrame, UnboundedFollowing, UnboundedPreceding, UnspecifiedFrame}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
    +import org.apache.spark.sql.catalyst.rules.RuleExecutor
    +
    +
    +class TransposeWindowSuite extends PlanTest {
    +  object Optimize extends RuleExecutor[LogicalPlan] {
    +    val batches =
    +      Batch("CollapseProject", FixedPoint(100), CollapseProject, RemoveRedundantProject) ::
    +      Batch("FlipWindow", Once, CollapseWindow, TransposeWindow) :: Nil
    +  }
    +
    +  val testRelation = LocalRelation('a.string, 'b.string, 'c.int, 'd.string)
    +
    +  val a = testRelation.output(0)
    +  val b = testRelation.output(1)
    +  val c = testRelation.output(2)
    +  val d = testRelation.output(3)
    +
    +  val partitionSpec1 = Seq(a)
    +  val partitionSpec2 = Seq(a, b)
    +  val partitionSpec3 = Seq(d)
    +
    +  val orderSpec1 = Seq(d.asc)
    +  val orderSpec2 = Seq(d.desc)
    +
    +  test("flip two adjacent windows with compatible partitions in multiple selects") {
    --- End diff --
    
    Agreed. I'll remove it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #76591 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76591/testReport)** for PR 17899 at commit [`f90c857`](https://github.com/apache/spark/commit/f90c857f0b06cfd5a7be0f44e2cc991554fe4cde).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116718847
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -609,6 +610,19 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is a subset of the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if ps1.length < ps2.length && ps2.containsSlice(ps1) =>
    --- End diff --
    
    We might be able to get a little more milage out of the rule by using `semanticEquals` for comparing the partition expressions, e.g.:
    ```scala
    def sliceSemanticEquals(ps1: Seq[Expression], ps2: Seq[Expression]): Boolean = ps1.zip(ps2).forall {
      case (l, r) => l.semanticEquals(r)
    }
    ...
    sliceSemanticEquals(ps1, ps2)
    ``` 
    
    You could even get more leverage if you do not consider the order of the partition spec.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    ok to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    **[Test build #77126 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77126/testReport)** for PR 17899 at commit [`f472bfe`](https://github.com/apache/spark/commit/f472bfecfcc008b3837aa1ecb903e02bbf665c9e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to flip a...

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

    https://github.com/apache/spark/pull/17899#discussion_r116719142
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/TransposeWindowSuite.scala ---
    @@ -0,0 +1,101 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.dsl.expressions._
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.expressions.{RowFrame, SpecifiedWindowFrame, UnboundedFollowing, UnboundedPreceding, UnspecifiedFrame}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
    +import org.apache.spark.sql.catalyst.rules.RuleExecutor
    +
    +
    +class TransposeWindowSuite extends PlanTest {
    +  object Optimize extends RuleExecutor[LogicalPlan] {
    +    val batches =
    +      Batch("CollapseProject", FixedPoint(100), CollapseProject, RemoveRedundantProject) ::
    +      Batch("FlipWindow", Once, CollapseWindow, TransposeWindow) :: Nil
    +  }
    +
    +  val testRelation = LocalRelation('a.string, 'b.string, 'c.int, 'd.string)
    +
    +  val a = testRelation.output(0)
    +  val b = testRelation.output(1)
    +  val c = testRelation.output(2)
    +  val d = testRelation.output(3)
    +
    +  val partitionSpec1 = Seq(a)
    +  val partitionSpec2 = Seq(a, b)
    +  val partitionSpec3 = Seq(d)
    +
    +  val orderSpec1 = Seq(d.asc)
    +  val orderSpec2 = Seq(d.desc)
    +
    +  test("flip two adjacent windows with compatible partitions in multiple selects") {
    +    val wexpr1 = windowExpr(sum('c), windowSpec(partitionSpec2, Seq.empty, UnspecifiedFrame))
    +    val wexpr2 = windowExpr(sum('c), windowSpec(partitionSpec1, Seq.empty, UnspecifiedFrame))
    +
    +    val query = testRelation
    +      .select('a, 'b, 'c, wexpr1.as('sum_a_2))
    +      .select('a, 'b, 'c, 'sum_a_2, wexpr2.as('sum_a_1))
    +
    +    val optimized = Optimize.execute(query.analyze)
    +
    +    val query2 = testRelation
    +      .select('a, 'b, 'c)
    +      .select('a, 'b, 'c, wexpr2.as('sum_a_1))
    +      .select('a, 'b, 'c, wexpr1.as('sum_a_2), 'sum_a_1)
    +      .select('a, 'b, 'c, 'sum_a_2, 'sum_a_1)
    +
    +    val correctAnswer = Optimize.execute(query2.analyze)
    +
    +    comparePlans(optimized, correctAnswer)
    +  }
    +
    +  test("flip two adjacent windows with compatible partitions") {
    +    val query = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec2, orderSpec2)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, orderSpec1)
    +
    +    val analyzed = query.analyze
    +    val optimized = Optimize.execute(analyzed)
    +
    +    val correctAnswer = testRelation
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, orderSpec1)
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec2, orderSpec2)
    +      .select('a, 'b, 'c, 'd, 'sum_a_2, 'sum_a_1)
    +
    +    comparePlans(optimized, correctAnswer.analyze)
    +  }
    +
    +  test("don't flip two adjacent windows with incompatible partitions") {
    +    val query = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec3, Seq.empty)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, Seq.empty)
    +
    +    val analyzed = query.analyze
    +    val optimized = Optimize.execute(analyzed)
    +
    +    val correctAnswer = testRelation
    +      .window(Seq(sum(c).as('sum_a_2)), partitionSpec3, Seq.empty)
    +      .window(Seq(sum(c).as('sum_a_1)), partitionSpec1, Seq.empty)
    +
    +    comparePlans(optimized, correctAnswer.analyze)
    --- End diff --
    
    `comparePlans(optimized, analyzed)`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to flip adjacent...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

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


---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to transp...

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

    https://github.com/apache/spark/pull/17899#discussion_r125013340
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -610,6 +611,25 @@ object CollapseWindow extends Rule[LogicalPlan] {
     }
     
     /**
    + * Transpose Adjacent Window Expressions.
    + * - If the partition spec of the parent Window expression is compatible with the partition spec
    + *   of the child window expression, transpose them.
    + */
    +object TransposeWindow extends Rule[LogicalPlan] {
    +  private def compatibleParititions(ps1 : Seq[Expression], ps2: Seq[Expression]): Boolean = {
    +    ps1.length < ps2.length && ps2.take(ps1.length).permutations.exists(ps1.zip(_).forall {
    +      case (l, r) => l.semanticEquals(r)
    +    })
    +  }
    +
    +  def apply(plan: LogicalPlan): LogicalPlan = plan transformUp {
    +    case w1 @ Window(we1, ps1, os1, w2 @ Window(we2, ps2, os2, grandChild))
    +        if w1.references.intersect(w2.windowOutputSet).isEmpty && compatibleParititions(ps1, ps2) =>
    --- End diff --
    
    I will add one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #17899: [SPARK-20636] Add new optimization rule to transpose adj...

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

    https://github.com/apache/spark/pull/17899
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #17899: [SPARK-20636] Add new optimization rule to transp...

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

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


---

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