You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/03/11 09:52:36 UTC
[GitHub] [ignite] tledkov-gridgain opened a new pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
tledkov-gridgain opened a new pull request #8869:
URL: https://github.com/apache/ignite/pull/8869
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] tledkov-gridgain closed pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain closed pull request #8869:
URL: https://github.com/apache/ignite/pull/8869
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #8869:
URL: https://github.com/apache/ignite/pull/8869#discussion_r595245404
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/HashAggregateConverterRule.java
##########
@@ -26,29 +26,85 @@
import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
/**
*
*/
-public class HashAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+public class HashAggregateConverterRule {
/** */
- public static final RelOptRule INSTANCE = new HashAggregateConverterRule();
+ public static final RelOptRule SINGLE = new HashSingleAggregateConverterRule();
/** */
- public HashAggregateConverterRule() {
- super(LogicalAggregate.class, "HashAggregateConverterRule");
+ public static final RelOptRule MAP_REDUCE = new HashMapReduceAggregateConverterRule();
+
+ /** */
+ private HashAggregateConverterRule() {
+ // No-op.
+ }
+
+ /** */
+ private static class HashSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ return new IgniteSingleHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
+ }
}
- /** {@inheritDoc} */
- @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
- LogicalAggregate rel) {
- RelOptCluster cluster = rel.getCluster();
- RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelNode input = convert(rel.getInput(), inTrait);
+ /** */
+ private static class HashMapReduceAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashMapReduceAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashMapReduceAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ RelNode map = new IgniteMapHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
Review comment:
input already converted
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AggregatePlannerTest.java
##########
@@ -223,42 +222,51 @@ public void mapReduceDistinctWithIndex() throws Exception {
enum AggregateAlgorithm {
/** */
SORT(
- IgniteSortAggregate.class,
+ IgniteSingleSortAggregate.class,
IgniteMapSortAggregate.class,
IgniteReduceSortAggregate.class,
- "HashAggregateConverterRule"
+ new String[] {"HashSingleAggregateConverterRule", "HashMapReduceAggregateConverterRule"},
+ new String[] {"HashSingleAggregateConverterRule", "HashMapReduceAggregateConverterRule",
+ "SortSingleAggregateConverterRule"}
),
/** */
HASH(
- IgniteHashAggregate.class,
+ IgniteSingleHashAggregate.class,
IgniteMapHashAggregate.class,
IgniteReduceHashAggregate.class,
- "SortAggregateConverterRule"
+ new String[] {"SortSingleAggregateConverterRule", "SortMapReduceAggregateConverterRule"},
+ new String[] {"SortSingleAggregateConverterRule", "SortMapReduceAggregateConverterRule",
+ "HashSingleAggregateConverterRule"}
);
/** */
- public final Class<? extends IgniteAggregateBase> single;
+ public final Class<? extends IgniteSingleAggregateBase> single;
/** */
- public final Class<? extends IgniteAggregate> map;
+ public final Class<? extends IgniteMapAggregateBase> map;
/** */
public final Class<? extends IgniteReduceAggregateBase> reduce;
/** */
- public final String ruleToDisable;
+ public final String[] rulesToDisableOtherAlgorithm;
+
+ /** */
+ public final String[] rulesToDisableOtherAlgorithmAndSingle;
Review comment:
i'd rather provide two more values of enum (SORT_SINGLE and HASH_SINGLE), and leave only one collection with name `rulesToDisable`
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/HashAggregateConverterRule.java
##########
@@ -26,29 +26,85 @@
import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
/**
*
*/
-public class HashAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+public class HashAggregateConverterRule {
/** */
- public static final RelOptRule INSTANCE = new HashAggregateConverterRule();
+ public static final RelOptRule SINGLE = new HashSingleAggregateConverterRule();
/** */
- public HashAggregateConverterRule() {
- super(LogicalAggregate.class, "HashAggregateConverterRule");
+ public static final RelOptRule MAP_REDUCE = new HashMapReduceAggregateConverterRule();
+
+ /** */
+ private HashAggregateConverterRule() {
+ // No-op.
+ }
+
+ /** */
+ private static class HashSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ return new IgniteSingleHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
+ }
}
- /** {@inheritDoc} */
- @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
- LogicalAggregate rel) {
- RelOptCluster cluster = rel.getCluster();
- RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelNode input = convert(rel.getInput(), inTrait);
+ /** */
+ private static class HashMapReduceAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashMapReduceAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashMapReduceAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ RelNode map = new IgniteMapHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
- return new IgniteHashAggregate(cluster, outTrait, input,
- rel.getGroupSet(), rel.getGroupSets(), rel.getAggCallList());
+ return new IgniteReduceHashAggregate(
+ cluster,
+ outTrait.replace(IgniteDistributions.single()),
+ convert(map, inTrait.replace(IgniteDistributions.single())),
Review comment:
please create `map` with a required traits and don't use `convert` here
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/HashAggregateConverterRule.java
##########
@@ -26,29 +26,85 @@
import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
/**
*
*/
-public class HashAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+public class HashAggregateConverterRule {
/** */
- public static final RelOptRule INSTANCE = new HashAggregateConverterRule();
+ public static final RelOptRule SINGLE = new HashSingleAggregateConverterRule();
/** */
- public HashAggregateConverterRule() {
- super(LogicalAggregate.class, "HashAggregateConverterRule");
+ public static final RelOptRule MAP_REDUCE = new HashMapReduceAggregateConverterRule();
+
+ /** */
+ private HashAggregateConverterRule() {
+ // No-op.
+ }
+
+ /** */
+ private static class HashSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ return new IgniteSingleHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
Review comment:
input already converted
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/SortAggregateConverterRule.java
##########
@@ -29,40 +29,108 @@
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.util.ImmutableIntList;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
import org.apache.ignite.internal.util.typedef.F;
-/** */
-public class SortAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+/**
+ *
+ */
+public class SortAggregateConverterRule {
+ /** */
+ public static final RelOptRule SINGLE = new SortSingleAggregateConverterRule();
+
+ /** */
+ public static final RelOptRule MAP_REDUCE = new SortMapReduceAggregateConverterRule();
+
/** */
- public static final RelOptRule INSTANCE = new SortAggregateConverterRule();
+ private SortAggregateConverterRule() {
+ // No-op.
+ }
/** */
- public SortAggregateConverterRule() {
- super(LogicalAggregate.class, "SortAggregateConverterRule");
+ private static class SortSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ SortSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "SortSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ // Applicable only for GROUP BY
+ if (F.isEmpty(agg.getGroupSet()) || agg.getGroupSets().size() > 1)
+ return null;
+
+ RelOptCluster cluster = agg.getCluster();
+ RelNode input = agg.getInput();
+
+ RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(agg.getGroupSet().asList()));
+
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation)
+ .replace(IgniteDistributions.single());
+
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation)
+ .replace(IgniteDistributions.single());
+
+ return new IgniteSingleSortAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
+ }
}
- /** {@inheritDoc} */
- @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
- LogicalAggregate rel) {
- // Applicable only for GROUP BY
- if (F.isEmpty(rel.getGroupSet()) || rel.getGroupSets().size() > 1)
- return null;
-
- RelOptCluster cluster = rel.getCluster();
- RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelNode input = rel.getInput();
-
- RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(rel.getGroupSet().asList()));
-
- return new IgniteSortAggregate(
- cluster,
- outTrait.replace(collation),
- convert(input, inTrait.replace(collation)),
- rel.getGroupSet(),
- rel.getGroupSets(),
- rel.getAggCallList()
- );
+ /** */
+ private static class SortMapReduceAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ SortMapReduceAggregateConverterRule() {
+ super(LogicalAggregate.class, "SortMapReduceAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ // Applicable only for GROUP BY
+ if (F.isEmpty(agg.getGroupSet()) || agg.getGroupSets().size() > 1)
+ return null;
+
+ RelOptCluster cluster = agg.getCluster();
+ RelNode input = agg.getInput();
+
+ RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(agg.getGroupSet().asList()));
+
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(collation);
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation);
+
+ RelNode map = new IgniteMapSortAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList(),
+ collation
+ );
+
+ return new IgniteReduceSortAggregate(
+ cluster,
+ outTrait.replace(IgniteDistributions.single()),
+ convert(map, inTrait.replace(IgniteDistributions.single())),
Review comment:
the same recommendation as for hash aggregate rule
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #8869:
URL: https://github.com/apache/ignite/pull/8869#discussion_r595360717
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AggregatePlannerTest.java
##########
@@ -223,42 +222,51 @@ public void mapReduceDistinctWithIndex() throws Exception {
enum AggregateAlgorithm {
/** */
SORT(
- IgniteSortAggregate.class,
+ IgniteSingleSortAggregate.class,
IgniteMapSortAggregate.class,
IgniteReduceSortAggregate.class,
- "HashAggregateConverterRule"
+ new String[] {"HashSingleAggregateConverterRule", "HashMapReduceAggregateConverterRule"},
+ new String[] {"HashSingleAggregateConverterRule", "HashMapReduceAggregateConverterRule",
+ "SortSingleAggregateConverterRule"}
),
/** */
HASH(
- IgniteHashAggregate.class,
+ IgniteSingleHashAggregate.class,
IgniteMapHashAggregate.class,
IgniteReduceHashAggregate.class,
- "SortAggregateConverterRule"
+ new String[] {"SortSingleAggregateConverterRule", "SortMapReduceAggregateConverterRule"},
+ new String[] {"SortSingleAggregateConverterRule", "SortMapReduceAggregateConverterRule",
+ "HashSingleAggregateConverterRule"}
);
/** */
- public final Class<? extends IgniteAggregateBase> single;
+ public final Class<? extends IgniteSingleAggregateBase> single;
/** */
- public final Class<? extends IgniteAggregate> map;
+ public final Class<? extends IgniteMapAggregateBase> map;
/** */
public final Class<? extends IgniteReduceAggregateBase> reduce;
/** */
- public final String ruleToDisable;
+ public final String[] rulesToDisableOtherAlgorithm;
+
+ /** */
+ public final String[] rulesToDisableOtherAlgorithmAndSingle;
Review comment:
The test `mapReduceDistinctWithIndex` moved to separate class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #8869:
URL: https://github.com/apache/ignite/pull/8869#discussion_r595249220
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/HashAggregateConverterRule.java
##########
@@ -26,29 +26,85 @@
import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleHashAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
/**
*
*/
-public class HashAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+public class HashAggregateConverterRule {
/** */
- public static final RelOptRule INSTANCE = new HashAggregateConverterRule();
+ public static final RelOptRule SINGLE = new HashSingleAggregateConverterRule();
/** */
- public HashAggregateConverterRule() {
- super(LogicalAggregate.class, "HashAggregateConverterRule");
+ public static final RelOptRule MAP_REDUCE = new HashMapReduceAggregateConverterRule();
+
+ /** */
+ private HashAggregateConverterRule() {
+ // No-op.
+ }
+
+ /** */
+ private static class HashSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(IgniteDistributions.single());
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ return new IgniteSingleHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
+ }
}
- /** {@inheritDoc} */
- @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
- LogicalAggregate rel) {
- RelOptCluster cluster = rel.getCluster();
- RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelNode input = convert(rel.getInput(), inTrait);
+ /** */
+ private static class HashMapReduceAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ HashMapReduceAggregateConverterRule() {
+ super(LogicalAggregate.class, "HashMapReduceAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ RelOptCluster cluster = agg.getCluster();
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
+ RelNode input = convert(agg.getInput(), inTrait);
+
+ RelNode map = new IgniteMapHashAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
- return new IgniteHashAggregate(cluster, outTrait, input,
- rel.getGroupSet(), rel.getGroupSets(), rel.getAggCallList());
+ return new IgniteReduceHashAggregate(
+ cluster,
+ outTrait.replace(IgniteDistributions.single()),
+ convert(map, inTrait.replace(IgniteDistributions.single())),
Review comment:
please create `map` with a required traits and don't use `convert` here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #8869: IGNITE-14274 Calcite integration: refactoring aggregates converter rules: single and map/reduce
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #8869:
URL: https://github.com/apache/ignite/pull/8869#discussion_r595284712
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/SortAggregateConverterRule.java
##########
@@ -29,40 +29,108 @@
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.util.ImmutableIntList;
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
-import org.apache.ignite.internal.processors.query.calcite.rel.IgniteSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteMapSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteReduceSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.agg.IgniteSingleSortAggregate;
+import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
import org.apache.ignite.internal.util.typedef.F;
-/** */
-public class SortAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+/**
+ *
+ */
+public class SortAggregateConverterRule {
+ /** */
+ public static final RelOptRule SINGLE = new SortSingleAggregateConverterRule();
+
+ /** */
+ public static final RelOptRule MAP_REDUCE = new SortMapReduceAggregateConverterRule();
+
/** */
- public static final RelOptRule INSTANCE = new SortAggregateConverterRule();
+ private SortAggregateConverterRule() {
+ // No-op.
+ }
/** */
- public SortAggregateConverterRule() {
- super(LogicalAggregate.class, "SortAggregateConverterRule");
+ private static class SortSingleAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ SortSingleAggregateConverterRule() {
+ super(LogicalAggregate.class, "SortSingleAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ // Applicable only for GROUP BY
+ if (F.isEmpty(agg.getGroupSet()) || agg.getGroupSets().size() > 1)
+ return null;
+
+ RelOptCluster cluster = agg.getCluster();
+ RelNode input = agg.getInput();
+
+ RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(agg.getGroupSet().asList()));
+
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation)
+ .replace(IgniteDistributions.single());
+
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation)
+ .replace(IgniteDistributions.single());
+
+ return new IgniteSingleSortAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList()
+ );
+ }
}
- /** {@inheritDoc} */
- @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
- LogicalAggregate rel) {
- // Applicable only for GROUP BY
- if (F.isEmpty(rel.getGroupSet()) || rel.getGroupSets().size() > 1)
- return null;
-
- RelOptCluster cluster = rel.getCluster();
- RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE);
- RelNode input = rel.getInput();
-
- RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(rel.getGroupSet().asList()));
-
- return new IgniteSortAggregate(
- cluster,
- outTrait.replace(collation),
- convert(input, inTrait.replace(collation)),
- rel.getGroupSet(),
- rel.getGroupSets(),
- rel.getAggCallList()
- );
+ /** */
+ private static class SortMapReduceAggregateConverterRule extends AbstractIgniteConverterRule<LogicalAggregate> {
+ /** */
+ SortMapReduceAggregateConverterRule() {
+ super(LogicalAggregate.class, "SortMapReduceAggregateConverterRule");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq,
+ LogicalAggregate agg) {
+ // Applicable only for GROUP BY
+ if (F.isEmpty(agg.getGroupSet()) || agg.getGroupSets().size() > 1)
+ return null;
+
+ RelOptCluster cluster = agg.getCluster();
+ RelNode input = agg.getInput();
+
+ RelCollation collation = RelCollations.of(ImmutableIntList.copyOf(agg.getGroupSet().asList()));
+
+ RelTraitSet inTrait = cluster.traitSetOf(IgniteConvention.INSTANCE).replace(collation);
+ RelTraitSet outTrait = cluster.traitSetOf(IgniteConvention.INSTANCE)
+ .replace(collation);
+
+ RelNode map = new IgniteMapSortAggregate(
+ cluster,
+ outTrait,
+ convert(input, inTrait),
+ agg.getGroupSet(),
+ agg.getGroupSets(),
+ agg.getAggCallList(),
+ collation
+ );
+
+ return new IgniteReduceSortAggregate(
+ cluster,
+ outTrait.replace(IgniteDistributions.single()),
+ convert(map, inTrait.replace(IgniteDistributions.single())),
Review comment:
the same recommendation as for hash aggregate rule
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org