Jackie-Jiang commented on code in PR #14664:
URL: https://github.com/apache/pinot/pull/14664#discussion_r1899317112
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java:
##########
@@ -83,48 +85,148 @@
* - COUNT(*)__LEAF produces TUPLE[ SUM(1), GROUP_BY_KEY ]
* - COUNT(*)__FINAL produces TUPLE[ SUM(COUNT(*)__LEAF), GROUP_BY_KEY ]
*/
-public class PinotAggregateExchangeNodeInsertRule extends RelOptRule {
- public static final PinotAggregateExchangeNodeInsertRule INSTANCE =
- new
PinotAggregateExchangeNodeInsertRule(PinotRuleUtils.PINOT_REL_FACTORY);
-
- public PinotAggregateExchangeNodeInsertRule(RelBuilderFactory factory) {
- // NOTE: Explicitly match for LogicalAggregate because after applying the
rule, LogicalAggregate is replaced with
- // PinotLogicalAggregate, and the rule won't be applied again.
- super(operand(LogicalAggregate.class, any()), factory, null);
+public class PinotAggregateExchangeNodeInsertRule {
+
+ public static class SortProjectAggregate extends RelOptRule {
+ public static final SortProjectAggregate INSTANCE = new
SortProjectAggregate(PinotRuleUtils.PINOT_REL_FACTORY);
+
+ private SortProjectAggregate(RelBuilderFactory factory) {
+ // NOTE: Explicitly match for LogicalAggregate because after applying
the rule, LogicalAggregate is replaced with
+ // PinotLogicalAggregate, and the rule won't be applied again.
+ super(operand(Sort.class, operand(Project.class,
operand(LogicalAggregate.class, any()))), factory, null);
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ // Apply this rule for group-by queries with enable group trim hint.
+ LogicalAggregate aggRel = call.rel(2);
+ if (aggRel.getGroupSet().isEmpty()) {
+ return;
+ }
+ Map<String, String> hintOptions =
+ PinotHintStrategyTable.getHintOptions(aggRel.getHints(),
PinotHintOptions.AGGREGATE_HINT_OPTIONS);
+ if (hintOptions == null || !Boolean.parseBoolean(
+
hintOptions.get(PinotHintOptions.AggregateOptions.ENABLE_GROUP_TRIM))) {
+ return;
+ }
+
+ Sort sortRel = call.rel(0);
+ Project projectRel = call.rel(1);
+ List<RexNode> projects = projectRel.getProjects();
+ List<RelFieldCollation> collations =
sortRel.getCollation().getFieldCollations();
+ if (collations.isEmpty()) {
+ // Cannot enable group trim without sort key.
Review Comment:
Limit can be pushed down with or without sorting. Initially I decided to not
allow pushing down limit without sorting because it can cause wrong result for
aggregate group-by, but realized this can prevent distinct limit to be pushed
down. Since this is not enabled by default (requires hint), I'll change it to
not check collation. I'm thinking also rename the hint to `is_push_down_limit`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
