You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2015/09/02 02:09:52 UTC

[16/18] incubator-calcite git commit: [CALCITE-827] Calcite incorrectly permutes columns of OVER query (Hsuan-Yi Chu)

[CALCITE-827] Calcite incorrectly permutes columns of OVER query (Hsuan-Yi Chu)

Close apache/incubator-calcite#115


Project: http://git-wip-us.apache.org/repos/asf/incubator-calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/b9670731
Tree: http://git-wip-us.apache.org/repos/asf/incubator-calcite/tree/b9670731
Diff: http://git-wip-us.apache.org/repos/asf/incubator-calcite/diff/b9670731

Branch: refs/heads/master
Commit: b9670731e05465fa2a257bab809f9ff71e7213cf
Parents: 5e7d457
Author: Hsuan-Yi Chu <hs...@usc.edu>
Authored: Sun Aug 2 17:29:33 2015 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Sep 1 16:17:16 2015 -0700

----------------------------------------------------------------------
 .../calcite/rel/logical/LogicalWindow.java      | 33 +++++++++++++++-----
 .../apache/calcite/test/RelOptRulesTest.java    | 19 +++++++++++
 .../org/apache/calcite/test/RelOptRulesTest.xml | 24 ++++++++++++++
 core/src/test/resources/sql/winagg.oq           | 23 ++++++++++++++
 4 files changed, 91 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/b9670731/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java b/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
index c1f4a02..2e65115 100644
--- a/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
+++ b/core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
@@ -48,6 +48,7 @@ import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -116,11 +117,13 @@ public final class LogicalWindow extends Window {
     // Build a list of groups, partitions, and aggregate functions. Each
     // aggregate function will add its arguments as outputs of the input
     // program.
+    final Map<RexOver, RexOver> origToNewOver = new IdentityHashMap<>();
     for (RexNode agg : program.getExprList()) {
       if (agg instanceof RexOver) {
-        RexOver over = (RexOver) agg;
-        over = (RexOver) over.accept(replaceConstants);
-        addWindows(windowMap, over, inputFieldCount);
+        final RexOver origOver = (RexOver) agg;
+        final RexOver newOver = (RexOver) origOver.accept(replaceConstants);
+        origToNewOver.put(origOver, newOver);
+        addWindows(windowMap, newOver, inputFieldCount);
       }
     }
 
@@ -199,7 +202,7 @@ public final class LogicalWindow extends Window {
           public RexNode visitOver(RexOver over) {
             // Look up the aggCall which this expr was translated to.
             final Window.RexWinAggCall aggCall =
-                aggMap.get(over);
+                aggMap.get(origToNewOver.get(over));
             assert aggCall != null;
             assert RelOptUtil.eq(
                 "over",
@@ -238,18 +241,32 @@ public final class LogicalWindow extends Window {
                 localRef.getType());
           }
         };
-    // TODO: The order that the "over" calls occur in the groups and
-    // partitions may not match the order in which they occurred in the
-    // original expression. We should add a project to permute them.
 
     LogicalWindow window =
         new LogicalWindow(
             cluster, traitSet, child, constants, intermediateRowType,
             groups);
 
+    // The order that the "over" calls occur in the groups and
+    // partitions may not match the order in which they occurred in the
+    // original expression.
+    // Add a project to permute them.
+    final List<RexNode> rexNodesWindow = new ArrayList<>();
+    for (RexNode rexNode : program.getExprList()) {
+      rexNodesWindow.add(rexNode.accept(shuttle));
+    }
+    final List<RexNode> refToWindow = toInputRefs(rexNodesWindow);
+
+    final List<RexNode> projectList = new ArrayList<>();
+    for (RexLocalRef inputRef : program.getProjectList()) {
+      final int index = inputRef.getIndex();
+      final RexInputRef ref = (RexInputRef) refToWindow.get(index);
+      projectList.add(ref);
+    }
+
     return RelOptUtil.createProject(
         window,
-        toInputRefs(program.getProjectList()),
+        projectList,
         outRowType.getFieldNames());
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/b9670731/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 5b60fb4..8c1f678 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -62,6 +62,7 @@ import org.apache.calcite.rel.rules.ProjectMergeRule;
 import org.apache.calcite.rel.rules.ProjectRemoveRule;
 import org.apache.calcite.rel.rules.ProjectSetOpTransposeRule;
 import org.apache.calcite.rel.rules.ProjectToCalcRule;
+import org.apache.calcite.rel.rules.ProjectToWindowRule;
 import org.apache.calcite.rel.rules.PruneEmptyRules;
 import org.apache.calcite.rel.rules.ReduceExpressionsRule;
 import org.apache.calcite.rel.rules.SemiJoinFilterTransposeRule;
@@ -139,6 +140,24 @@ public class RelOptRulesTest extends RelOptTestBase {
     return DiffRepository.lookup(RelOptRulesTest.class);
   }
 
+  @Test public void testProjectToWindowRuleForMultipleWindows() {
+    HepProgram preProgram =  new HepProgramBuilder()
+        .build();
+
+    HepProgramBuilder builder = new HepProgramBuilder();
+    builder.addRuleClass(ProjectToWindowRule.class);
+    HepPlanner hepPlanner = new HepPlanner(builder.build());
+    hepPlanner.addRule(ProjectToWindowRule.PROJECT);
+
+    checkPlanning(tester,
+        preProgram,
+        hepPlanner,
+        "select count(*) over(partition by empno order by sal) as count1,\n"
+            + "count(*) over(partition by deptno order by sal) as count2, \n"
+            + "sum(deptno) over(partition by empno order by sal) as sum1, \n"
+            + "sum(deptno) over(partition by deptno order by sal) as sum2 \n"
+            + "from emp");
+  }
 
   @Test public void testUnionToDistinctRule() {
     checkPlanning(UnionToDistinctRule.INSTANCE,

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/b9670731/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index bd650ce..542f153 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -16,6 +16,30 @@ See the License for the specific language governing permissions and
 limitations under the License.
 -->
 <Root>
+    <TestCase name="testProjectToWindowRuleForMultipleWindows">
+        <Resource name="sql">
+            <![CDATA[
+select count(*) over(partition by empno order by sal) as count1,
+    count(*) over(partition by deptno order by sal) as count2,
+    sum(deptno)  over(partition by empno order by sal) as sum1,
+    sum(deptno)  over(partition by deptno order by sal) as sum2
+    from emp
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject($0=[$9], $1=[$11], $2=[$10], $3=[$12])
+  LogicalWindow(window#0=[window(partition {0} order by [5] range between UNBOUNDED PRECEDING and CURRENT ROW aggs [COUNT(), SUM($7)])], window#1=[window(partition {7} order by [5] range between UNBOUNDED PRECEDING and CURRENT ROW aggs [COUNT(), SUM($7)])])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(COUNT1=[COUNT() OVER (PARTITION BY $0 ORDER BY $5 RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)], COUNT2=[COUNT() OVER (PARTITION BY $7 ORDER BY $5 RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)], SUM1=[SUM($7) OVER (PARTITION BY $0 ORDER BY $5 RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)], SUM2=[SUM($7) OVER (PARTITION BY $7 ORDER BY $5 RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)])
+  LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testUnionToDistinctRule">
         <Resource name="sql">
             <![CDATA[select * from dept union select * from dept]]>

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/b9670731/core/src/test/resources/sql/winagg.oq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/winagg.oq b/core/src/test/resources/sql/winagg.oq
index 590af75..44bc186 100644
--- a/core/src/test/resources/sql/winagg.oq
+++ b/core/src/test/resources/sql/winagg.oq
@@ -17,6 +17,29 @@
 #
 !use post
 !set outputformat psql
+
+# Multiple window functions sharing a single window
+select count(*) over(partition by gender order by ename) as count1,
+  count(*) over(partition by deptno order by ename) as count2,
+  sum(deptno) over(partition by gender order by ename) as sum1,
+  sum(deptno) over(partition by deptno order by ename) as sum2
+from emp
+order by sum1, sum2;
+ COUNT1 | COUNT2 | SUM1 | SUM2
+--------+--------+------+------
+      1 |      1 |   30 |   30
+      1 |      1 |   50 |   50
+      2 |      1 |   60 |   10
+      3 |      1 |   80 |   20
+      2 |      2 |   80 |  100
+      3 |      1 |  140 |   60
+      4 |      2 |  150 |   20
+      5 |      2 |  180 |   60
+      6 |      1 |  180 |     
+(9 rows)
+
+!ok
+
 !if (false) {
 select *, first_value(deptno) over () from emp;
  ename | deptno | gender | first_value