Repository: calcite Updated Branches: refs/heads/master 5149568ce -> b8e09fe9a
[CALCITE-948] Indicator columns not preserved by RelFieldTrimmer Close apache/calcite#163 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/b8e09fe9 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/b8e09fe9 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/b8e09fe9 Branch: refs/heads/master Commit: b8e09fe9a2e22a82065107d56649bf86e6edbafb Parents: 5149568 Author: Jesus Camacho Rodriguez <[email protected]> Authored: Tue Nov 3 21:49:44 2015 +0100 Committer: Jesus Camacho Rodriguez <[email protected]> Committed: Tue Nov 3 21:49:44 2015 +0100 ---------------------------------------------------------------------- .../AggregateExpandDistinctAggregatesRule.java | 5 +++-- .../rel/rules/AggregateJoinTransposeRule.java | 5 +++-- .../AggregateProjectPullUpConstantsRule.java | 4 ++-- .../rel/rules/AggregateReduceFunctionsRule.java | 1 + .../rel/rules/AggregateUnionAggregateRule.java | 2 +- .../rel/rules/AggregateUnionTransposeRule.java | 4 ++-- .../apache/calcite/sql2rel/RelFieldTrimmer.java | 3 ++- .../java/org/apache/calcite/tools/RelBuilder.java | 18 ++++++++++-------- .../org/apache/calcite/test/RelBuilderTest.java | 8 ++++---- 9 files changed, 28 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java index 96449e9..e76c7aa 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java @@ -188,7 +188,7 @@ public final class AggregateExpandDistinctAggregatesRule extends RelOptRule { int n = 0; if (!newAggCallList.isEmpty()) { final RelBuilder.GroupKey groupKey = - relBuilder.groupKey(groupSet, aggregate.getGroupSets()); + relBuilder.groupKey(groupSet, aggregate.indicator, aggregate.getGroupSets()); relBuilder.aggregate(groupKey, newAggCallList); ++n; } @@ -228,7 +228,7 @@ public final class AggregateExpandDistinctAggregatesRule extends RelOptRule { final RelBuilder relBuilder = call.builder(); relBuilder.push(aggregate.getInput()); - relBuilder.aggregate(relBuilder.groupKey(fullGroupSet, groupSets), + relBuilder.aggregate(relBuilder.groupKey(fullGroupSet, groupSets.size() > 1, groupSets), distinctAggCalls); final RelNode distinct = relBuilder.peek(); final int groupCount = fullGroupSet.cardinality(); @@ -332,6 +332,7 @@ public final class AggregateExpandDistinctAggregatesRule extends RelOptRule { relBuilder.aggregate( relBuilder.groupKey( remap(fullGroupSet, aggregate.getGroupSet()), + aggregate.indicator, remap(fullGroupSet, aggregate.getGroupSets())), newCalls); relBuilder.convert(aggregate.getRowType(), true); http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java index 0f4d733..016e45a 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java @@ -247,7 +247,7 @@ public class AggregateJoinTransposeRule extends RelOptRule { } } side.newInput = relBuilder.push(joinInput) - .aggregate(relBuilder.groupKey(belowAggregateKey, null), + .aggregate(relBuilder.groupKey(belowAggregateKey, false, null), belowAggCalls) .build(); } @@ -331,7 +331,8 @@ public class AggregateJoinTransposeRule extends RelOptRule { } relBuilder.aggregate( relBuilder.groupKey(Mappings.apply(mapping, aggregate.getGroupSet()), - Mappings.apply2(mapping, aggregate.getGroupSets())), newAggCalls); + aggregate.indicator, Mappings.apply2(mapping, aggregate.getGroupSets())), + newAggCalls); } call.transformTo(relBuilder.build()); } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java index 94a28eb..2b99cf6 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java @@ -136,7 +136,7 @@ public class AggregateProjectPullUpConstantsRule extends RelOptRule { groupCount, newGroupCount)); } relBuilder.aggregate( - relBuilder.groupKey(ImmutableBitSet.range(newGroupCount), null), + relBuilder.groupKey(ImmutableBitSet.range(newGroupCount), false, null), newAggCalls); } else { // Create the mapping from old field positions to new field @@ -180,7 +180,7 @@ public class AggregateProjectPullUpConstantsRule extends RelOptRule { // Aggregate on projection. relBuilder.aggregate( - relBuilder.groupKey(ImmutableBitSet.range(newGroupCount), null), + relBuilder.groupKey(ImmutableBitSet.range(newGroupCount), false, null), newAggCalls); } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java index f01d738..c1f2f7f 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java @@ -537,6 +537,7 @@ public class AggregateReduceFunctionsRule extends RelOptRule { List<AggregateCall> newCalls) { relBuilder.aggregate( relBuilder.groupKey(oldAggregate.getGroupSet(), + oldAggregate.indicator, oldAggregate.getGroupSets()), newCalls); } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionAggregateRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionAggregateRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionAggregateRule.java index 8eeaaa7..a99a29e 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionAggregateRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionAggregateRule.java @@ -104,7 +104,7 @@ public class AggregateUnionAggregateRule extends RelOptRule { } relBuilder.union(true); - relBuilder.aggregate(relBuilder.groupKey(topAggRel.getGroupSet(), null), + relBuilder.aggregate(relBuilder.groupKey(topAggRel.getGroupSet(), false, null), topAggRel.getAggCallList()); call.transformTo(relBuilder.build()); } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionTransposeRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionTransposeRule.java index e1f1396..2a2ef98 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateUnionTransposeRule.java @@ -124,7 +124,7 @@ public class AggregateUnionTransposeRule extends RelOptRule { relBuilder.push(input); if (!alreadyUnique) { ++transformCount; - relBuilder.aggregate(relBuilder.groupKey(aggRel.getGroupSet(), null), + relBuilder.aggregate(relBuilder.groupKey(aggRel.getGroupSet(), false, null), aggRel.getAggCallList()); } } @@ -139,7 +139,7 @@ public class AggregateUnionTransposeRule extends RelOptRule { // create a new union whose children are the aggregates created above relBuilder.union(true, union.getInputs().size()); relBuilder.aggregate( - relBuilder.groupKey(aggRel.getGroupSet(), aggRel.getGroupSets()), + relBuilder.groupKey(aggRel.getGroupSet(), aggRel.indicator, aggRel.getGroupSets()), transformedAggCalls); call.transformTo(relBuilder.build()); } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java index 2f0218b..5c82a96 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java @@ -830,7 +830,8 @@ public class RelFieldTrimmer implements ReflectiveVisitor { ++j; } - final RelBuilder.GroupKey groupKey = relBuilder.groupKey(newGroupSet, newGroupSets); + final RelBuilder.GroupKey groupKey = relBuilder.groupKey(newGroupSet, + aggregate.indicator, newGroupSets); relBuilder.aggregate(groupKey, newAggCallList); return new TrimResult(relBuilder.build(), mapping); http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/main/java/org/apache/calcite/tools/RelBuilder.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java index 5e6ea45..e2400b8 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -540,18 +540,18 @@ public class RelBuilder { /** Creates a group key. */ public GroupKey groupKey(Iterable<? extends RexNode> nodes) { - return new GroupKeyImpl(ImmutableList.copyOf(nodes), null, null); + return new GroupKeyImpl(ImmutableList.copyOf(nodes), false, null, null); } /** Creates a group key with grouping sets. */ - public GroupKey groupKey(Iterable<? extends RexNode> nodes, + public GroupKey groupKey(Iterable<? extends RexNode> nodes, boolean indicator, Iterable<? extends Iterable<? extends RexNode>> nodeLists) { final ImmutableList.Builder<ImmutableList<RexNode>> builder = ImmutableList.builder(); for (Iterable<? extends RexNode> nodeList : nodeLists) { builder.add(ImmutableList.copyOf(nodeList)); } - return new GroupKeyImpl(ImmutableList.copyOf(nodes), builder.build(), null); + return new GroupKeyImpl(ImmutableList.copyOf(nodes), indicator, builder.build(), null); } /** Creates a group key of fields identified by ordinal. */ @@ -570,7 +570,7 @@ public class RelBuilder { * <p>This method of creating a group key does not allow you to group on new * expressions, only column projections, but is efficient, especially when you * are coming from an existing {@link Aggregate}. */ - public GroupKey groupKey(ImmutableBitSet groupSet, + public GroupKey groupKey(ImmutableBitSet groupSet, boolean indicator, ImmutableList<ImmutableBitSet> groupSets) { if (groupSet.length() > peek().getRowType().getFieldCount()) { throw new IllegalArgumentException("out of bounds: " + groupSet); @@ -587,7 +587,7 @@ public class RelBuilder { return fields(ImmutableIntList.of(input.toArray())); } }); - return groupKey(nodes, nodeLists); + return groupKey(nodes, indicator, nodeLists); } /** Creates a call to an aggregate function. */ @@ -852,7 +852,7 @@ public class RelBuilder { assert groupSet.contains(set); } RelNode aggregate = aggregateFactory.createAggregate(r, - groupSets.size() > 1, groupSet, groupSets, aggregateCalls); + groupKey_.indicator, groupSet, groupSets, aggregateCalls); push(aggregate); return this; } @@ -1366,12 +1366,14 @@ public class RelBuilder { /** Implementation of {@link RelBuilder.GroupKey}. */ protected static class GroupKeyImpl implements GroupKey { final ImmutableList<RexNode> nodes; + final boolean indicator; final ImmutableList<ImmutableList<RexNode>> nodeLists; final String alias; - GroupKeyImpl(ImmutableList<RexNode> nodes, + GroupKeyImpl(ImmutableList<RexNode> nodes, boolean indicator, ImmutableList<ImmutableList<RexNode>> nodeLists, String alias) { this.nodes = Preconditions.checkNotNull(nodes); + this.indicator = indicator; this.nodeLists = nodeLists; this.alias = alias; } @@ -1383,7 +1385,7 @@ public class RelBuilder { public GroupKey alias(String alias) { return Objects.equals(this.alias, alias) ? this - : new GroupKeyImpl(nodes, nodeLists, alias); + : new GroupKeyImpl(nodes, indicator, nodeLists, alias); } } http://git-wip-us.apache.org/repos/asf/calcite/blob/b8e09fe9/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index 7ce40c8..84b42fd 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -412,7 +412,7 @@ public class RelBuilderTest { RelNode root = builder.scan("EMP") .aggregate( - builder.groupKey(ImmutableBitSet.of(7), + builder.groupKey(ImmutableBitSet.of(7), true, ImmutableList.of(ImmutableBitSet.of(7), ImmutableBitSet.of())), builder.aggregateCall(SqlStdOperatorTable.COUNT, false, @@ -431,7 +431,7 @@ public class RelBuilderTest { try { RelNode root = builder.scan("EMP") - .aggregate(builder.groupKey(ImmutableBitSet.of(17), null)) + .aggregate(builder.groupKey(ImmutableBitSet.of(17), false, null)) .build(); fail("expected error, got " + root); } catch (IllegalArgumentException e) { @@ -445,7 +445,7 @@ public class RelBuilderTest { RelNode root = builder.scan("EMP") .aggregate( - builder.groupKey(ImmutableBitSet.of(7), + builder.groupKey(ImmutableBitSet.of(7), true, ImmutableList.of(ImmutableBitSet.of(4), ImmutableBitSet.of()))) .build(); @@ -461,7 +461,7 @@ public class RelBuilderTest { RelNode root = builder.scan("EMP") .aggregate( - builder.groupKey(ImmutableBitSet.of(7, 6), + builder.groupKey(ImmutableBitSet.of(7, 6), true, ImmutableList.of(ImmutableBitSet.of(7), ImmutableBitSet.of(6), ImmutableBitSet.of(7))))
