calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jh...@apache.org
Subject [1/2] calcite git commit: [CALCITE-1097] Exception when executing query with too many aggregation columns (chenzhifa)
Date Sun, 21 Feb 2016 08:20:56 GMT
Repository: calcite
Updated Branches:
  refs/heads/master 19cfde74d -> f4b4ee115


[CALCITE-1097] Exception when executing query with too many aggregation columns (chenzhifa)

Change the SyntheticRecordType class generated method to avoid using too many parameters.

Close apache/calcite#198


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

Branch: refs/heads/master
Commit: 5fdc866b72739163a6a418ea7aa1a7f68812761e
Parents: 19cfde7
Author: chenzhifa <chenzhifa@mininglamp.com>
Authored: Sat Feb 20 22:08:56 2016 +0800
Committer: Julian Hyde <jhyde@apache.org>
Committed: Sat Feb 20 12:08:40 2016 -0800

----------------------------------------------------------------------
 .../adapter/enumerable/EnumerableAggregate.java |  32 ++-
 .../enumerable/EnumerableRelImplementor.java    |  15 +-
 .../java/org/apache/calcite/test/JdbcTest.java  |  50 +++++
 .../java/org/apache/calcite/util/Smalls.java    | 223 +++++++++++++++++++
 4 files changed, 307 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/5fdc866b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java
index 60f9b11..76a0501 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java
@@ -19,6 +19,7 @@ package org.apache.calcite.adapter.enumerable;
 import org.apache.calcite.adapter.enumerable.impl.AggAddContextImpl;
 import org.apache.calcite.adapter.enumerable.impl.AggResultContextImpl;
 import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
 import org.apache.calcite.linq4j.Ord;
 import org.apache.calcite.linq4j.function.Function0;
 import org.apache.calcite.linq4j.function.Function1;
@@ -27,6 +28,7 @@ import org.apache.calcite.linq4j.tree.BlockBuilder;
 import org.apache.calcite.linq4j.tree.Expression;
 import org.apache.calcite.linq4j.tree.Expressions;
 import org.apache.calcite.linq4j.tree.ParameterExpression;
+import org.apache.calcite.linq4j.tree.Types;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.prepare.CalcitePrepareImpl;
@@ -252,7 +254,35 @@ public class EnumerableAggregate extends Aggregate implements EnumerableRel
{
             typeFactory,
             typeFactory.createSyntheticType(aggStateTypes));
 
-    initBlock.add(accPhysType.record(initExpressions));
+
+    if (accPhysType.getJavaRowType() instanceof JavaTypeFactoryImpl.SyntheticRecordType)
{
+      // We have to initialize the SyntheticRecordType instance this way, to avoid using
+      // class constructor with too many parameters.
+      JavaTypeFactoryImpl.SyntheticRecordType synType =
+        (JavaTypeFactoryImpl.SyntheticRecordType) accPhysType.getJavaRowType();
+      final ParameterExpression record0_ =
+        Expressions.parameter(accPhysType.getJavaRowType(), "record0");
+      initBlock.add(Expressions.declare(0, record0_, null));
+      initBlock.add(
+        Expressions.statement(
+          Expressions.assign(
+            record0_,
+            Expressions.new_(accPhysType.getJavaRowType()))));
+      List<Types.RecordField> fieldList = synType.getRecordFields();
+      for (int i = 0; i < initExpressions.size(); i++) {
+        Expression right = initExpressions.get(i);
+        initBlock.add(
+          Expressions.statement(
+            Expressions.assign(
+              Expressions.field(
+                record0_,
+                fieldList.get(i)),
+              right)));
+      }
+      initBlock.add(record0_);
+    } else {
+      initBlock.add(accPhysType.record(initExpressions));
+    }
 
     final Expression accumulatorInitializer =
         builder.append(

http://git-wip-us.apache.org/repos/asf/calcite/blob/5fdc866b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
index bd77f2b..bc88333 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelImplementor.java
@@ -185,18 +185,9 @@ public class EnumerableRelImplementor extends JavaRelImplementor {
     final List<ParameterExpression> parameters = new ArrayList<>();
     final ParameterExpression thisParameter =
         Expressions.parameter(type, "this");
-    for (Types.RecordField field : type.getRecordFields()) {
-      final ParameterExpression parameter =
-          Expressions.parameter(field.getType(), field.getName());
-      parameters.add(parameter);
-      blockBuilder.add(
-          Expressions.statement(
-              Expressions.assign(
-                  Expressions.field(
-                      thisParameter,
-                      field),
-                  parameter)));
-    }
+
+    // Here a constructor without parameter is used because the generated
+    // code could cause error if number of fields is too large.
     classDeclaration.memberDeclarations.add(
         Expressions.constructorDecl(
             Modifier.PUBLIC,

http://git-wip-us.apache.org/repos/asf/calcite/blob/5fdc866b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 0cc70ba..fc4ca15 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -6345,6 +6345,56 @@ public class JdbcTest {
             });
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1097">[CALCITE-1097]
+   * Exception when executing query with too many aggregation columns</a>. */
+  @Test public void testAggMultipleMeasures() throws SQLException {
+    final Driver driver = new Driver();
+    CalciteConnection connection = (CalciteConnection)
+        driver.connect("jdbc:calcite:", new Properties());
+    SchemaPlus rootSchema = connection.getRootSchema();
+    rootSchema.add("sale", new ReflectiveSchema(new Smalls.WideSaleSchema()));
+    connection.setSchema("sale");
+    final Statement statement = connection.createStatement();
+
+    // 200 columns: sum(sale0) + ... sum(sale199)
+    ResultSet resultSet =
+        statement.executeQuery("select s.\"prodId\"" + sums(200, true) + "\n"
+            + "from \"sale\".\"prod\" as s group by s.\"prodId\"\n");
+    assertThat(resultSet.next(), is(true));
+    assertThat(resultSet.getInt(1), is(100));
+    assertThat(resultSet.getInt(2), is(10));
+    assertThat(resultSet.getInt(200), is(10));
+    assertThat(resultSet.next(), is(false));
+
+    // 800 columns:
+    //   sum(sale0 + 0) + ... + sum(sale0 + 100) + ... sum(sale99 + 799)
+    final int n = 800;
+    resultSet =
+        statement.executeQuery("select s.\"prodId\"" + sums(n, false) + "\n"
+            + "from \"sale\".\"prod\" as s group by s.\"prodId\"\n");
+    assertThat(resultSet.next(), is(true));
+    assertThat(resultSet.getInt(1), is(100));
+    assertThat(resultSet.getInt(2), is(10));
+    assertThat(resultSet.getInt(n), is(n + 8));
+    assertThat(resultSet.next(), is(false));
+
+    connection.close();
+  }
+
+  private static String sums(int n, boolean c) {
+    final StringBuilder b = new StringBuilder();
+    for (int i = 0; i < n; i++) {
+      if (c) {
+        b.append(", sum(s.\"sale").append(i).append("\")");
+      } else {
+        b.append(", sum(s.\"sale").append(i % 100).append("\"").append(" + ")
+            .append(i).append(")");
+      }
+    }
+    return b.toString();
+  }
+
   // Disable checkstyle, so it doesn't complain about fields like "customer_id".
   //CHECKSTYLE: OFF
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/5fdc866b/core/src/test/java/org/apache/calcite/util/Smalls.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/util/Smalls.java b/core/src/test/java/org/apache/calcite/util/Smalls.java
index d9e07ef..b6fa4f8 100644
--- a/core/src/test/java/org/apache/calcite/util/Smalls.java
+++ b/core/src/test/java/org/apache/calcite/util/Smalls.java
@@ -585,6 +585,229 @@ public class Smalls {
       return Linq4j.asEnumerable(rows);
     }
   }
+
+  /** Schema containing a {@code prod} table with a lot of columns. */
+  public static class WideSaleSchema {
+    @Override public String toString() {
+      return "WideSaleSchema";
+    }
+
+    @SuppressWarnings("unused")
+    public final WideProductSale[] prod = {
+      new WideProductSale(100, 10)
+    };
+  }
+
+  /** Table with a lot of columns. */
+  @SuppressWarnings("unused")
+  public static class WideProductSale {
+    public final int prodId;
+    public final double sale0;
+    public final double sale1 = 10;
+    public final double sale2 = 10;
+    public final double sale3 = 10;
+    public final double sale4 = 10;
+    public final double sale5 = 10;
+    public final double sale6 = 10;
+    public final double sale7 = 10;
+    public final double sale8 = 10;
+    public final double sale9 = 10;
+    public final double sale10 = 10;
+    public final double sale11 = 10;
+    public final double sale12 = 10;
+    public final double sale13 = 10;
+    public final double sale14 = 10;
+    public final double sale15 = 10;
+    public final double sale16 = 10;
+    public final double sale17 = 10;
+    public final double sale18 = 10;
+    public final double sale19 = 10;
+    public final double sale20 = 10;
+    public final double sale21 = 10;
+    public final double sale22 = 10;
+    public final double sale23 = 10;
+    public final double sale24 = 10;
+    public final double sale25 = 10;
+    public final double sale26 = 10;
+    public final double sale27 = 10;
+    public final double sale28 = 10;
+    public final double sale29 = 10;
+    public final double sale30 = 10;
+    public final double sale31 = 10;
+    public final double sale32 = 10;
+    public final double sale33 = 10;
+    public final double sale34 = 10;
+    public final double sale35 = 10;
+    public final double sale36 = 10;
+    public final double sale37 = 10;
+    public final double sale38 = 10;
+    public final double sale39 = 10;
+    public final double sale40 = 10;
+    public final double sale41 = 10;
+    public final double sale42 = 10;
+    public final double sale43 = 10;
+    public final double sale44 = 10;
+    public final double sale45 = 10;
+    public final double sale46 = 10;
+    public final double sale47 = 10;
+    public final double sale48 = 10;
+    public final double sale49 = 10;
+    public final double sale50 = 10;
+    public final double sale51 = 10;
+    public final double sale52 = 10;
+    public final double sale53 = 10;
+    public final double sale54 = 10;
+    public final double sale55 = 10;
+    public final double sale56 = 10;
+    public final double sale57 = 10;
+    public final double sale58 = 10;
+    public final double sale59 = 10;
+    public final double sale60 = 10;
+    public final double sale61 = 10;
+    public final double sale62 = 10;
+    public final double sale63 = 10;
+    public final double sale64 = 10;
+    public final double sale65 = 10;
+    public final double sale66 = 10;
+    public final double sale67 = 10;
+    public final double sale68 = 10;
+    public final double sale69 = 10;
+    public final double sale70 = 10;
+    public final double sale71 = 10;
+    public final double sale72 = 10;
+    public final double sale73 = 10;
+    public final double sale74 = 10;
+    public final double sale75 = 10;
+    public final double sale76 = 10;
+    public final double sale77 = 10;
+    public final double sale78 = 10;
+    public final double sale79 = 10;
+    public final double sale80 = 10;
+    public final double sale81 = 10;
+    public final double sale82 = 10;
+    public final double sale83 = 10;
+    public final double sale84 = 10;
+    public final double sale85 = 10;
+    public final double sale86 = 10;
+    public final double sale87 = 10;
+    public final double sale88 = 10;
+    public final double sale89 = 10;
+    public final double sale90 = 10;
+    public final double sale91 = 10;
+    public final double sale92 = 10;
+    public final double sale93 = 10;
+    public final double sale94 = 10;
+    public final double sale95 = 10;
+    public final double sale96 = 10;
+    public final double sale97 = 10;
+    public final double sale98 = 10;
+    public final double sale99 = 10;
+    public final double sale100 = 10;
+    public final double sale101 = 10;
+    public final double sale102 = 10;
+    public final double sale103 = 10;
+    public final double sale104 = 10;
+    public final double sale105 = 10;
+    public final double sale106 = 10;
+    public final double sale107 = 10;
+    public final double sale108 = 10;
+    public final double sale109 = 10;
+    public final double sale110 = 10;
+    public final double sale111 = 10;
+    public final double sale112 = 10;
+    public final double sale113 = 10;
+    public final double sale114 = 10;
+    public final double sale115 = 10;
+    public final double sale116 = 10;
+    public final double sale117 = 10;
+    public final double sale118 = 10;
+    public final double sale119 = 10;
+    public final double sale120 = 10;
+    public final double sale121 = 10;
+    public final double sale122 = 10;
+    public final double sale123 = 10;
+    public final double sale124 = 10;
+    public final double sale125 = 10;
+    public final double sale126 = 10;
+    public final double sale127 = 10;
+    public final double sale128 = 10;
+    public final double sale129 = 10;
+    public final double sale130 = 10;
+    public final double sale131 = 10;
+    public final double sale132 = 10;
+    public final double sale133 = 10;
+    public final double sale134 = 10;
+    public final double sale135 = 10;
+    public final double sale136 = 10;
+    public final double sale137 = 10;
+    public final double sale138 = 10;
+    public final double sale139 = 10;
+    public final double sale140 = 10;
+    public final double sale141 = 10;
+    public final double sale142 = 10;
+    public final double sale143 = 10;
+    public final double sale144 = 10;
+    public final double sale145 = 10;
+    public final double sale146 = 10;
+    public final double sale147 = 10;
+    public final double sale148 = 10;
+    public final double sale149 = 10;
+    public final double sale150 = 10;
+    public final double sale151 = 10;
+    public final double sale152 = 10;
+    public final double sale153 = 10;
+    public final double sale154 = 10;
+    public final double sale155 = 10;
+    public final double sale156 = 10;
+    public final double sale157 = 10;
+    public final double sale158 = 10;
+    public final double sale159 = 10;
+    public final double sale160 = 10;
+    public final double sale161 = 10;
+    public final double sale162 = 10;
+    public final double sale163 = 10;
+    public final double sale164 = 10;
+    public final double sale165 = 10;
+    public final double sale166 = 10;
+    public final double sale167 = 10;
+    public final double sale168 = 10;
+    public final double sale169 = 10;
+    public final double sale170 = 10;
+    public final double sale171 = 10;
+    public final double sale172 = 10;
+    public final double sale173 = 10;
+    public final double sale174 = 10;
+    public final double sale175 = 10;
+    public final double sale176 = 10;
+    public final double sale177 = 10;
+    public final double sale178 = 10;
+    public final double sale179 = 10;
+    public final double sale180 = 10;
+    public final double sale181 = 10;
+    public final double sale182 = 10;
+    public final double sale183 = 10;
+    public final double sale184 = 10;
+    public final double sale185 = 10;
+    public final double sale186 = 10;
+    public final double sale187 = 10;
+    public final double sale188 = 10;
+    public final double sale189 = 10;
+    public final double sale190 = 10;
+    public final double sale191 = 10;
+    public final double sale192 = 10;
+    public final double sale193 = 10;
+    public final double sale194 = 10;
+    public final double sale195 = 10;
+    public final double sale196 = 10;
+    public final double sale197 = 10;
+    public final double sale198 = 10;
+    public final double sale199 = 10;
+
+    public WideProductSale(int prodId, double sale) {
+      this.prodId = prodId;
+      this.sale0 = sale;
+    }
+  }
 }
 
 // End Smalls.java


Mime
View raw message