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 <[email protected]> Authored: Sat Feb 20 22:08:56 2016 +0800 Committer: Julian Hyde <[email protected]> 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
