morrySnow commented on code in PR #14397: URL: https://github.com/apache/doris/pull/14397#discussion_r1092987998
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/WindowFunction.java: ########## @@ -0,0 +1,64 @@ +// 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; + +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.BoundFunction; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.VarcharType; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; + +/** + * Window functions, as known as analytic functions. + */ +public abstract class WindowFunction extends BoundFunction { + + public WindowFunction(String name, Expression... arguments) { + super(name, arguments); + } + + public WindowFunction(String name, List<Expression> children) { + super(name, children); + } + + protected List<DataType> intermediateTypes() { + return ImmutableList.of(VarcharType.SYSTEM_DEFAULT); + } Review Comment: window function do not need intermediateTypes i think ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/Ntile.java: ########## @@ -0,0 +1,58 @@ +// 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; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable; +import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; +import org.apache.doris.nereids.trees.expressions.shape.LeafExpression; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.IntegerType; + +/** + * window function: Ntile() + */ +public class Ntile extends WindowFunction implements LeafExpression, AlwaysNotNullable, CustomSignature { Review Comment: why use CustomSignature here? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalQuickSort.java: ########## @@ -44,20 +44,19 @@ public PhysicalQuickSort(List<OrderKey> orderKeys, } /** - * Constructor of PhysicalHashJoinNode. + * Constructor of PhysicalQuickSort. Review Comment: do not change the indent of this file~ ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/FirstOrLastValue.java: ########## @@ -0,0 +1,47 @@ +// 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; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; +import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable; +import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression; +import org.apache.doris.nereids.types.DataType; + +/** parent class for first_value() and last_value() */ +public class FirstOrLastValue extends WindowFunction implements UnaryExpression, PropagateNullable, CustomSignature { Review Comment: abstract ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/Lag.java: ########## @@ -0,0 +1,88 @@ +// 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; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.ImplicitlyCastableSignature; +import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable; +import org.apache.doris.nereids.trees.expressions.literal.Literal; +import org.apache.doris.nereids.trees.expressions.shape.TernaryExpression; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.IntegerType; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import java.util.List; + +/** Window function: lag */ +public class Lag extends WindowFunction implements TernaryExpression, PropagateNullable, ImplicitlyCastableSignature { + + public Lag(Expression child) { + this(child, Literal.of(1), Literal.of(null)); + } + + public Lag(Expression child, Expression offset) { + this(child, offset, Literal.of(null)); + } + + public Lag(Expression child, Expression offset, Expression defaultValue) { + super("lag", child, offset, defaultValue); + } + + public Lag(List<Expression> children) { + super("lag", children); + } + + public Expression getOffset() { + return child(1); + } + + public Expression getDefaultValue() { + return child(2); + } + + public void setDefaultValue(Expression defaultValue) { + this.children.set(2, defaultValue); + } Review Comment: our expression is immutable, so should use withChildren to update default value ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/Lead.java: ########## @@ -0,0 +1,91 @@ +// 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; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; +import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable; +import org.apache.doris.nereids.trees.expressions.literal.Literal; +import org.apache.doris.nereids.trees.expressions.shape.TernaryExpression; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.DataType; + +import com.google.common.base.Preconditions; + +import java.util.List; + +/** + * Window function: Lead() + */ +public class Lead extends WindowFunction implements TernaryExpression, PropagateNullable, ExplicitlyCastableSignature { + + public Lead(Expression child) { + this(child, Literal.of(1), Literal.of(null)); + } + + public Lead(Expression child, Expression offset) { + this(child, offset, Literal.of(null)); + } + + public Lead(Expression child, Expression offset, Expression defaultValue) { + super("lead", child, offset, defaultValue); + } + + public Lead(List<Expression> children) { + super("lead", children); + } + + public Expression getOffset() { + return child(1); + } + + public Expression getDefaultValue() { + return child(2); + } + + public void setDefaultValue(Expression defaultValue) { + this.children.set(2, defaultValue); + } + + @Override + public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) { + return visitor.visitLead(this, context); + } + + @Override + public List<FunctionSignature> getSignatures() { + return null; + } + + @Override + public FunctionSignature searchSignature(List<FunctionSignature> signatures) { + return null; + } Review Comment: why return null? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java: ########## @@ -611,4 +623,35 @@ private StatsDeriveResult computeGenerate(Generate generate) { } return new StatsDeriveResult(count, columnStatsMap); } + + private StatsDeriveResult computeWindow(Window windowOperator) { + StatsDeriveResult stats = groupExpression.childStatistics(0); + Map<Id, ColumnStatistic> columnStatisticMap = new HashMap<>(); + Map<Id, ColumnStatistic> childColumnStats = stats.getSlotIdToColumnStats(); + windowOperator.getOutputExpressions().stream() + .map(expr -> { + ColumnStatistic value = null; + Set<Slot> slots = expr.getInputSlots(); + if (slots.isEmpty()) { + value = ColumnStatistic.DEFAULT; + } else { + for (Slot slot : slots) { + if (childColumnStats.containsKey(slot.getExprId())) { + value = childColumnStats.get(slot.getExprId()); + break; + } + } + if (value == null) { + // process window expression + // todo: how to set stats? + // ColumnStatisticBuilder builder = new ColumnStatisticBuilder().setNdv(1) + // .setDataSize(expr.getDataType().width()); Review Comment: remove useless comments, except TODO ########## fe/fe-core/src/test/java/org/apache/doris/nereids/parser/NereidsParserTest.java: ########## @@ -106,6 +106,16 @@ public void testParseCTE() { Assertions.assertEquals(columnAliases.get().size(), 2); } + @Test + public void testParseWindowFunctions() { + NereidsParser nereidsParser = new NereidsParser(); + LogicalPlan logicalPlan; + + String windowSql1 = "select rank() over(partition by k1 order by k1) as ranking from t1"; + logicalPlan = nereidsParser.parseSingle(windowSql1); + System.out.println(logicalPlan.treeString()); Review Comment: should we need to check plan pattern? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/Ntile.java: ########## @@ -0,0 +1,58 @@ +// 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; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable; +import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; +import org.apache.doris.nereids.trees.expressions.shape.LeafExpression; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.IntegerType; + +/** + * window function: Ntile() + */ +public class Ntile extends WindowFunction implements LeafExpression, AlwaysNotNullable, CustomSignature { + + private int buckets; + + public Ntile(int buckets) { Review Comment: why not treat buckets as its child? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java: ########## @@ -611,4 +623,35 @@ private StatsDeriveResult computeGenerate(Generate generate) { } return new StatsDeriveResult(count, columnStatsMap); } + + private StatsDeriveResult computeWindow(Window windowOperator) { + StatsDeriveResult stats = groupExpression.childStatistics(0); + Map<Id, ColumnStatistic> columnStatisticMap = new HashMap<>(); + Map<Id, ColumnStatistic> childColumnStats = stats.getSlotIdToColumnStats(); + windowOperator.getOutputExpressions().stream() Review Comment: should assign return value to `columnStatisticMap`. -- 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
