morrySnow commented on code in PR #53037:
URL: https://github.com/apache/doris/pull/53037#discussion_r2196515152
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -4166,26 +4187,59 @@ private LogicalPlan withFilter(LogicalPlan input,
Optional<WhereClauseContext> w
}
private LogicalPlan withAggregate(LogicalPlan input,
SelectColumnClauseContext selectCtx,
- Optional<AggClauseContext> aggCtx) {
+ Optional<AggClauseContext> aggCtx, List<OrderKey> orderKeys) {
return input.optionalMap(aggCtx, () -> {
GroupingElementContext groupingElementContext =
aggCtx.get().groupingElement();
List<NamedExpression> namedExpressions =
getNamedExpressions(selectCtx.namedExpressionSeq());
if (groupingElementContext.GROUPING() != null) {
ImmutableList.Builder<List<Expression>> groupingSets =
ImmutableList.builder();
for (GroupingSetContext groupingSetContext :
groupingElementContext.groupingSet()) {
- groupingSets.add(visit(groupingSetContext.expression(),
Expression.class));
+ List<GroupKeyWithOrder> groupKeyWithOrders =
visit(groupingSetContext.expressionWithOrder(),
+ GroupKeyWithOrder.class);
+ ImmutableList<Expression> groupByExpressions =
groupKeyWithOrders.stream()
+ .map(GroupKeyWithOrder::getExpr)
+ .collect(ImmutableList.toImmutableList());
+ groupKeyWithOrders.stream()
+ .filter(GroupKeyWithOrder::hasOrder)
+ .map(e -> new OrderKey(e.getExpr(), e.isAsc(),
e.isAsc()))
Review Comment:
if any one has order, we should let all key as a part of order key, the
default is asc
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -4166,26 +4187,59 @@ private LogicalPlan withFilter(LogicalPlan input,
Optional<WhereClauseContext> w
}
private LogicalPlan withAggregate(LogicalPlan input,
SelectColumnClauseContext selectCtx,
- Optional<AggClauseContext> aggCtx) {
+ Optional<AggClauseContext> aggCtx, List<OrderKey> orderKeys) {
return input.optionalMap(aggCtx, () -> {
GroupingElementContext groupingElementContext =
aggCtx.get().groupingElement();
List<NamedExpression> namedExpressions =
getNamedExpressions(selectCtx.namedExpressionSeq());
if (groupingElementContext.GROUPING() != null) {
ImmutableList.Builder<List<Expression>> groupingSets =
ImmutableList.builder();
for (GroupingSetContext groupingSetContext :
groupingElementContext.groupingSet()) {
- groupingSets.add(visit(groupingSetContext.expression(),
Expression.class));
+ List<GroupKeyWithOrder> groupKeyWithOrders =
visit(groupingSetContext.expressionWithOrder(),
+ GroupKeyWithOrder.class);
+ ImmutableList<Expression> groupByExpressions =
groupKeyWithOrders.stream()
+ .map(GroupKeyWithOrder::getExpr)
+ .collect(ImmutableList.toImmutableList());
+ groupKeyWithOrders.stream()
+ .filter(GroupKeyWithOrder::hasOrder)
+ .map(e -> new OrderKey(e.getExpr(), e.isAsc(),
e.isAsc()))
+ .forEach(orderKeys::add);
+ groupingSets.add(groupByExpressions);
}
return new LogicalRepeat<>(groupingSets.build(),
namedExpressions, input);
} else if (groupingElementContext.CUBE() != null) {
- List<Expression> cubeExpressions =
visit(groupingElementContext.expression(), Expression.class);
+ List<GroupKeyWithOrder> groupKeyWithOrders =
visit(groupingElementContext.expressionWithOrder(),
Review Comment:
`cube` and `grouping sets` do not support it. `rollup` only the syntax
`group by a, b with rollup` need to support this feature.
--
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]
