morrySnow commented on code in PR #14397:
URL: https://github.com/apache/doris/pull/14397#discussion_r1092848340
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -945,15 +954,99 @@ public Expression
visitFunctionCall(DorisParser.FunctionCallContext ctx) {
if (!unboundStars.get(0).getQualifier().isEmpty()) {
throw new ParseException("'*' can not has qualifier: "
+ unboundStars.size(), ctx);
}
+ if (ctx.windowSpec() != null) {
+ throw new ParseException(
+ "COUNT(*) isn't supported as window function;
can use COUNT(col)", ctx);
+ }
return new Count();
}
throw new ParseException("'*' can only be used in conjunction
with COUNT: " + functionName, ctx);
} else {
- return new UnboundFunction(functionName, isDistinct, params);
+ UnboundFunction function = new UnboundFunction(functionName,
isDistinct, params);
+ if (ctx.windowSpec() != null) {
+ if (isDistinct) {
+ throw new ParseException("DISTINCT not allowed in
window function: " + functionName, ctx);
+ }
+ return withWindowSpec(ctx.windowSpec(), function);
+ }
+ return function;
}
});
}
+ /**
+ * deal with window function definition
+ */
+ private Window withWindowSpec(WindowSpecContext ctx, Expression function) {
+ Optional<List<Expression>> partitionKeys =
optionalVisit(ctx.partitionClause(),
+ () -> visit(ctx.partitionClause().expression(),
Expression.class));
+
+ Optional<List<OrderKey>> orderKeys = optionalVisit(ctx.sortClause(),
+ () -> visit(ctx.sortClause().sortItem(), OrderKey.class));
+
+ Optional<WindowFrame> windowFrame = optionalVisit(ctx.windowFrame(),
+ () -> withWindowFrame(ctx.windowFrame()));
+
+ return new Window(function, partitionKeys, orderKeys, windowFrame);
+ }
+
+ /**
+ * deal with optional expressions
+ */
+ private <T, C> Optional optionalVisit(T ctx, Supplier<C> func) {
+ return Optional.ofNullable(ctx).map(a -> Optional.of(func.get()))
+ .orElse(Optional.empty());
+ }
Review Comment:
```suggestion
private <T, C> Optional<C> optionalVisit(T ctx, Supplier<C> func) {
return Optional.ofNullable(ctx).map(a -> func.get());
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -340,13 +350,12 @@ private LogicalPlan withCte(LogicalPlan plan, CteContext
ctx) {
public LogicalSubQueryAlias<Plan> visitAliasQuery(AliasQueryContext ctx) {
return ParserUtils.withOrigin(ctx, () -> {
LogicalPlan queryPlan = plan(ctx.query());
- List<String> columnNames = null;
- if (ctx.columnAliases() != null) {
- columnNames = ctx.columnAliases().identifier().stream()
+ Optional<List<String>> columnNames =
optionalVisit(ctx.columnAliases(), () ->
+ ctx.columnAliases().identifier().stream()
.map(RuleContext::getText)
- .collect(ImmutableList.toImmutableList());
- }
- return new LogicalSubQueryAlias<>(ctx.identifier().getText(),
Optional.ofNullable(columnNames), queryPlan);
+ .collect(ImmutableList.toImmutableList())
+ );
+ return new LogicalSubQueryAlias(ctx.identifier().getText(),
columnNames, queryPlan);
Review Comment:
```suggestion
return new LogicalSubQueryAlias<>(ctx.identifier().getText(),
columnNames, queryPlan);
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -340,13 +350,12 @@ private LogicalPlan withCte(LogicalPlan plan, CteContext
ctx) {
public LogicalSubQueryAlias<Plan> visitAliasQuery(AliasQueryContext ctx) {
return ParserUtils.withOrigin(ctx, () -> {
LogicalPlan queryPlan = plan(ctx.query());
- List<String> columnNames = null;
- if (ctx.columnAliases() != null) {
- columnNames = ctx.columnAliases().identifier().stream()
+ Optional<List<String>> columnNames =
optionalVisit(ctx.columnAliases(), () ->
+ ctx.columnAliases().identifier().stream()
Review Comment:
```suggestion
Optional<List<String>> columnNames =
optionalVisit(ctx.columnAliases(),
() -> ctx.columnAliases().identifier().stream()
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/FrameUnitsType.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.window;
+
+/**
+ * frame units types
+ */
+public enum FrameUnitsType {
Review Comment:
since it is only used in WindowFrame, i think it is better to treat it as a
inner enum of `WindowFrame`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1437,12 +1530,19 @@ private LogicalPlan withProjection(LogicalPlan input,
SelectColumnClauseContext
expressions, input, isDistinct);
} else {
List<NamedExpression> projects =
getNamedExpressions(selectCtx.namedExpressionSeq());
- return new LogicalProject<>(projects, ImmutableList.of(),
input, isDistinct);
+ if (containsWindowExpressions(projects)) {
+ return new LogicalWindow<>(projects, input);
+ }
+ return new LogicalProject<>(projects,
Collections.emptyList(), input, isDistinct);
Review Comment:
maybe we should generate LogicalWindow after we do bind.
we chould just treat windowExpression as a scalar expression.
- if we use window function without agg. Then window function will in
Projections.
we could generate LogicalProject(LogicalWindow(LogicalProject)) to handle
all case.
- if we use window function in agg. Then window function will in Agg's
output expressions. After normalize agg it will present in the Project topping
on Aggregate. and then we could do the same thing as first scene.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/FrameUnitsType.java:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.window;
+
+/**
+ * frame units types
+ */
+public enum FrameUnitsType {
+
+ ROWS("ROWS"),
+ RANGE("RANGE");
+
+ private final String description;
Review Comment:
description is not use anymore.
--
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
