morrySnow commented on code in PR #12274: URL: https://github.com/apache/doris/pull/12274#discussion_r961587855
########## fe/fe-core/src/main/java/org/apache/doris/nereids/types/BooleanType.java: ########## @@ -26,6 +26,8 @@ public class BooleanType extends PrimitiveType { public static BooleanType INSTANCE = new BooleanType(); + private static int width = 1; + Review Comment: static variables need use upper case word with '_' as separator ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PruneAggChildColumns.java: ########## @@ -54,15 +58,57 @@ public class PruneAggChildColumns extends OneRewriteRuleFactory { @Override public Rule build() { return RuleType.COLUMN_PRUNE_AGGREGATION_CHILD.build(logicalAggregate().then(agg -> { + Slot slot = handleCountStar(agg); + List<Slot> childOutput = agg.child().getOutput(); + if (slot != null) { + if (childOutput.size() == 1 && childOutput.get(0).equals(slot)) { + return agg; + } + return agg.withChildren(ImmutableList.of(new LogicalProject<>(ImmutableList.of(slot), agg.child()))); + } List<Expression> slots = Lists.newArrayList(); slots.addAll(agg.getExpressions()); Set<Slot> outputs = SlotExtractor.extractSlot(slots); - List<NamedExpression> prunedOutputs = agg.child().getOutput().stream().filter(outputs::contains) + List<NamedExpression> prunedOutputs = childOutput.stream().filter(outputs::contains) .collect(Collectors.toList()); if (prunedOutputs.size() == agg.child().getOutput().size()) { return agg; } return agg.withChildren(ImmutableList.of(new LogicalProject<>(prunedOutputs, agg.child()))); })); } + + private Slot handleCountStar(LogicalAggregate<GroupPlan> agg) { + List<NamedExpression> outputExpressions = agg.getOutputExpressions(); + if (outputExpressions.size() != 1) { + return null; + } + NamedExpression namedExpression = outputExpressions.get(0); Review Comment: we need to collect all agg function from all output expressions and judge whether all of them have no child output slot reference as leaf node ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PruneAggChildColumns.java: ########## @@ -54,15 +58,57 @@ public class PruneAggChildColumns extends OneRewriteRuleFactory { @Override public Rule build() { return RuleType.COLUMN_PRUNE_AGGREGATION_CHILD.build(logicalAggregate().then(agg -> { + Slot slot = handleCountStar(agg); + List<Slot> childOutput = agg.child().getOutput(); + if (slot != null) { + if (childOutput.size() == 1 && childOutput.get(0).equals(slot)) { + return agg; + } + return agg.withChildren(ImmutableList.of(new LogicalProject<>(ImmutableList.of(slot), agg.child()))); + } List<Expression> slots = Lists.newArrayList(); slots.addAll(agg.getExpressions()); Set<Slot> outputs = SlotExtractor.extractSlot(slots); - List<NamedExpression> prunedOutputs = agg.child().getOutput().stream().filter(outputs::contains) + List<NamedExpression> prunedOutputs = childOutput.stream().filter(outputs::contains) .collect(Collectors.toList()); if (prunedOutputs.size() == agg.child().getOutput().size()) { return agg; } return agg.withChildren(ImmutableList.of(new LogicalProject<>(prunedOutputs, agg.child()))); })); } + + private Slot handleCountStar(LogicalAggregate<GroupPlan> agg) { Review Comment: we need a generalize method name, such as `handleNoChildOutput` ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PruneAggChildColumns.java: ########## @@ -54,15 +58,57 @@ public class PruneAggChildColumns extends OneRewriteRuleFactory { @Override public Rule build() { return RuleType.COLUMN_PRUNE_AGGREGATION_CHILD.build(logicalAggregate().then(agg -> { + Slot slot = handleCountStar(agg); + List<Slot> childOutput = agg.child().getOutput(); + if (slot != null) { + if (childOutput.size() == 1 && childOutput.get(0).equals(slot)) { + return agg; + } + return agg.withChildren(ImmutableList.of(new LogicalProject<>(ImmutableList.of(slot), agg.child()))); + } List<Expression> slots = Lists.newArrayList(); slots.addAll(agg.getExpressions()); Set<Slot> outputs = SlotExtractor.extractSlot(slots); - List<NamedExpression> prunedOutputs = agg.child().getOutput().stream().filter(outputs::contains) + List<NamedExpression> prunedOutputs = childOutput.stream().filter(outputs::contains) .collect(Collectors.toList()); if (prunedOutputs.size() == agg.child().getOutput().size()) { return agg; } return agg.withChildren(ImmutableList.of(new LogicalProject<>(prunedOutputs, agg.child()))); })); } + + private Slot handleCountStar(LogicalAggregate<GroupPlan> agg) { + List<NamedExpression> outputExpressions = agg.getOutputExpressions(); + if (outputExpressions.size() != 1) { Review Comment: need to handle ```sql select count(*), count(1), count(2) from t ``` -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org