korlov42 commented on a change in pull request #9829:
URL: https://github.com/apache/ignite/pull/9829#discussion_r817477211
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/SortAggregateConverterRule.java
##########
@@ -51,10 +51,10 @@ private SortAggregateConverterRule() {
}
/** */
- private static class SortSingleAggregateConverterRule extends
AbstractIgniteConverterRule<LogicalAggregate> {
+ private static class SortColocatedAggregateConverterRule extends
AbstractIgniteConverterRule<LogicalAggregate> {
Review comment:
Currently we have:
- IgniteColocatedHashAggregate
- IgniteColocatedSortAggregate
- IgniteMapHashAggregate
- IgniteMapSortAggregate
- IgniteReduceHashAggregate
- IgniteReduceSortAggregate
The pattern is `<some prefix><type of distribution><aggregation strategy (if
I may say so)>Aggregate`. But parts `distribution` and `strategy` are switched
in the convertors' names:
- SortColocatedAggregateConverterRule
- SortMapReduceAggregateConverterRule
- HashColocatedAggregateConverterRule
- HashMapReduceAggregateConverterRule
Probably, it would be better to rename converters to match the relations'
class names. WDYT?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
