[CALCITE-2647] In RelBuilder, add a groupKey method that assumes only one grouping set
Deprecate RelBuilder.groupKey(nodes, boolean, nodeList) (indicator is obsolete). Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/0b7b24a6 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/0b7b24a6 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/0b7b24a6 Branch: refs/heads/master Commit: 0b7b24a689d1791ad5c8127f02f69a3dba124689 Parents: b47413a Author: Julian Hyde <[email protected]> Authored: Fri Sep 28 14:51:24 2018 -0700 Committer: Julian Hyde <[email protected]> Committed: Wed Oct 31 12:00:51 2018 -0700 ---------------------------------------------------------------------- .../rel/rules/AbstractMaterializedViewRule.java | 4 +- .../rel/rules/AggregateExtractProjectRule.java | 7 ++- .../rel/rules/AggregateJoinTransposeRule.java | 3 +- .../AggregateProjectPullUpConstantsRule.java | 2 +- .../rel/rules/AggregateUnionAggregateRule.java | 2 +- .../rel/rules/AggregateUnionTransposeRule.java | 2 +- .../rel/rules/IntersectToDistinctRule.java | 2 +- .../org/apache/calcite/tools/RelBuilder.java | 51 ++++++++++++++++---- .../org/apache/calcite/test/RelBuilderTest.java | 2 +- site/_docs/algebra.md | 2 +- 10 files changed, 54 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java index cb34dc8..3a82f95 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java @@ -1204,7 +1204,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule { } RelNode prevNode = relBuilder.peek(); RelNode result = relBuilder - .aggregate(relBuilder.groupKey(groupSet, null), aggregateCalls) + .aggregate(relBuilder.groupKey(groupSet), aggregateCalls) .build(); if (prevNode == result && groupSet.cardinality() != result.getRowType().getFieldCount()) { // Aggregate was not inserted but we need to prune columns @@ -1486,7 +1486,7 @@ public abstract class AbstractMaterializedViewRule extends RelOptRule { relBuilder.project(inputViewExprs); } result = relBuilder - .aggregate(relBuilder.groupKey(groupSet, null), aggregateCalls) + .aggregate(relBuilder.groupKey(groupSet), aggregateCalls) .build(); if (prevNode == result && groupSet.cardinality() != result.getRowType().getFieldCount()) { // Aggregate was not inserted but we need to prune columns http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/core/src/main/java/org/apache/calcite/rel/rules/AggregateExtractProjectRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExtractProjectRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExtractProjectRule.java index 23e1425..5559c97 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExtractProjectRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExtractProjectRule.java @@ -105,10 +105,9 @@ public class AggregateExtractProjectRule extends RelOptRule { final ImmutableBitSet newGroupSet = Mappings.apply(mapping, aggregate.getGroupSet()); - final ImmutableList<ImmutableBitSet> newGroupSets = - ImmutableList.copyOf( - Iterables.transform(aggregate.getGroupSets(), - bitSet -> Mappings.apply(mapping, bitSet))); + final Iterable<ImmutableBitSet> newGroupSets = + Iterables.transform(aggregate.getGroupSets(), + bitSet -> Mappings.apply(mapping, bitSet)); final List<RelBuilder.AggCall> newAggCallList = new ArrayList<>(); for (AggregateCall aggCall : aggregate.getAggCallList()) { final ImmutableList<RexNode> args = http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 e841952..9d6cf67 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 @@ -285,8 +285,7 @@ public class AggregateJoinTransposeRule extends RelOptRule { } } side.newInput = relBuilder.push(joinInput) - .aggregate(relBuilder.groupKey(belowAggregateKey, null), - belowAggCalls) + .aggregate(relBuilder.groupKey(belowAggregateKey), belowAggCalls) .build(); } offset += fieldCount; http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 8686db2..a12e6d1 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 @@ -154,7 +154,7 @@ public class AggregateProjectPullUpConstantsRule extends RelOptRule { aggCall.adaptTo(input, aggCall.getArgList(), aggCall.filterArg, groupCount, newGroupCount)); } - relBuilder.aggregate(relBuilder.groupKey(newGroupSet, null), newAggCalls); + relBuilder.aggregate(relBuilder.groupKey(newGroupSet), newAggCalls); // Create a projection back again. List<Pair<RexNode, String>> projects = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 f65bb7f..5f1e22d 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 @@ -128,7 +128,7 @@ public class AggregateUnionAggregateRule extends RelOptRule { } relBuilder.union(true); - relBuilder.aggregate(relBuilder.groupKey(topAggRel.getGroupSet(), null), + relBuilder.aggregate(relBuilder.groupKey(topAggRel.getGroupSet()), topAggRel.getAggCallList()); call.transformTo(relBuilder.build()); } http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 1025753..d6fd60e 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 @@ -127,7 +127,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()), aggRel.getAggCallList()); } } http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java b/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java index 2b4bfbb..cf16557 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java @@ -107,7 +107,7 @@ public class IntersectToDistinctRule extends RelOptRule { final ImmutableBitSet groupSet = ImmutableBitSet.range(fieldCount - 1); - relBuilder.aggregate(relBuilder.groupKey(groupSet, null), + relBuilder.aggregate(relBuilder.groupKey(groupSet), relBuilder.countStar(null)); // add a filter count(c) = #branches http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 749fb9c..9a42e58 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -104,6 +104,7 @@ import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import javax.annotation.Nonnull; import static org.apache.calcite.util.Static.RESOURCE; @@ -664,8 +665,23 @@ public class RelBuilder { } /** Creates a group key with grouping sets. */ + public GroupKey groupKey(Iterable<? extends RexNode> nodes, + Iterable<? extends Iterable<? extends RexNode>> nodeLists) { + return groupKey_(nodes, false, nodeLists); + } + + /** @deprecated Now that indicator is deprecated, use + * {@link #groupKey(Iterable, Iterable)}, which has the same behavior as + * calling this method with {@code indicator = false}. */ + @Deprecated // to be removed before 2.0 public GroupKey groupKey(Iterable<? extends RexNode> nodes, boolean indicator, Iterable<? extends Iterable<? extends RexNode>> nodeLists) { + return groupKey_(nodes, indicator, nodeLists); + } + + private 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) { @@ -684,6 +700,16 @@ public class RelBuilder { return groupKey(fields(ImmutableList.copyOf(fieldNames))); } + /** Creates a group key, identified by field positions + * in the underlying relational expression. + * + * <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(@Nonnull ImmutableBitSet groupSet) { + return groupKey(groupSet, ImmutableList.of(groupSet)); + } + /** Creates a group key with grouping sets, both identified by field positions * in the underlying relational expression. * @@ -691,31 +717,38 @@ public class RelBuilder { * expressions, only column projections, but is efficient, especially when you * are coming from an existing {@link Aggregate}. */ public GroupKey groupKey(ImmutableBitSet groupSet, + @Nonnull Iterable<? extends ImmutableBitSet> groupSets) { + return groupKey_(groupSet, false, ImmutableList.copyOf(groupSets)); + } + + /** As {@link #groupKey(ImmutableBitSet, Iterable)}. */ + // deprecated, to be removed before 2.0 + public GroupKey groupKey(ImmutableBitSet groupSet, ImmutableList<ImmutableBitSet> groupSets) { - return groupKey_(groupSet, false, groupSets); + return groupKey_(groupSet, false, groupSets == null + ? ImmutableList.of(groupSet) : ImmutableList.copyOf(groupSets)); } - /** @deprecated Use {@link #groupKey(ImmutableBitSet, ImmutableList)}. */ + /** @deprecated Use {@link #groupKey(ImmutableBitSet, Iterable)}. */ @Deprecated // to be removed before 2.0 public GroupKey groupKey(ImmutableBitSet groupSet, boolean indicator, ImmutableList<ImmutableBitSet> groupSets) { - return groupKey_(groupSet, indicator, groupSets); + return groupKey_(groupSet, indicator, groupSets == null + ? ImmutableList.of(groupSet) : ImmutableList.copyOf(groupSets)); } private GroupKey groupKey_(ImmutableBitSet groupSet, boolean indicator, - ImmutableList<ImmutableBitSet> groupSets) { + @Nonnull ImmutableList<ImmutableBitSet> groupSets) { if (groupSet.length() > peek().getRowType().getFieldCount()) { throw new IllegalArgumentException("out of bounds: " + groupSet); } - if (groupSets == null) { - groupSets = ImmutableList.of(groupSet); - } + Objects.requireNonNull(groupSets); final ImmutableList<RexNode> nodes = fields(ImmutableIntList.of(groupSet.toArray())); final List<ImmutableList<RexNode>> nodeLists = - Lists.transform(groupSets, + Util.transform(groupSets, bitSet -> fields(ImmutableIntList.of(bitSet.toArray()))); - return groupKey(nodes, indicator, nodeLists); + return groupKey_(nodes, indicator, nodeLists); } @Deprecated // to be removed before 2.0 http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/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 c4a49fa..e89026a 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -838,7 +838,7 @@ public class RelBuilderTest { try { RelNode root = builder.scan("EMP") - .aggregate(builder.groupKey(ImmutableBitSet.of(17), null)) + .aggregate(builder.groupKey(ImmutableBitSet.of(17))) .build(); fail("expected error, got " + root); } catch (IllegalArgumentException e) { http://git-wip-us.apache.org/repos/asf/calcite/blob/0b7b24a6/site/_docs/algebra.md ---------------------------------------------------------------------- diff --git a/site/_docs/algebra.md b/site/_docs/algebra.md index 003255c..2b5c66a 100644 --- a/site/_docs/algebra.md +++ b/site/_docs/algebra.md @@ -381,7 +381,7 @@ The following methods return a |:------------------- |:----------- | `groupKey(fieldName...)`<br/>`groupKey(fieldOrdinal...)`<br/>`groupKey(expr...)`<br/>`groupKey(exprList)` | Creates a group key of the given expressions | `groupKey(exprList, exprListList)` | Creates a group key of the given expressions with grouping sets -| `groupKey(bitSet, bitSets)` | Creates a group key of the given input columns with grouping sets +| `groupKey(bitSet [, bitSets])` | Creates a group key of the given input columns, with multiple grouping sets if `bitSets` is specified ### Aggregate call methods
