morrySnow commented on code in PR #20164:
URL: https://github.com/apache/doris/pull/20164#discussion_r1213866018
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java:
##########
@@ -30,22 +30,28 @@
*/
public class AggStateType extends PrimitiveType {
- public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null);
+ public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null, null);
Review Comment:
do not use null
```suggestion
public static final AggStateType SYSTEM_DEFAULT = new AggStateType("",
ImmutableList.of(), ImmutableList.of());
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java:
##########
@@ -30,22 +30,28 @@
*/
public class AggStateType extends PrimitiveType {
- public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null);
+ public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null, null);
public static final int WIDTH = 16;
private final List<DataType> subTypes;
private final List<Boolean> subTypeNullables;
+ private final String functionName;
- public AggStateType(List<DataType> subTypes, List<Boolean>
subTypeNullables) {
+ public AggStateType(String functionName, List<DataType> subTypes,
List<Boolean> subTypeNullables) {
this.subTypes = subTypes;
Review Comment:
check the two list are same size
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
Review Comment:
why named the dir `combinator`, it is not clear to understand
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
Review Comment:
all attr must be final
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
Review Comment:
should combine these two static var into one named some like `MERGE_SUFFIX`
inside AggStateFunctionBuilder
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/StateCombinator.java:
##########
Review Comment:
same issues as MergeCombinator, please fix them too
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
+ Preconditions.checkState(nested instanceof AggregateFunction);
+ this.nested = (AggregateFunction) nested;
+ }
+
+ @Override
+ public MergeCombinator withChildren(List<Expression> children) {
+ throw new UnsupportedOperationException("Unimplemented method
'withChildren'");
+ }
+
+ @Override
+ public List<FunctionSignature> getSignatures() {
+ AggStateType type = new AggStateType(nested.getName(),
nested.getArgumentsTypes(),
+
nested.getArguments().stream().map(Expression::nullable).collect(Collectors.toList()));
+ return nested.getSignatures().stream().map(sig -> {
+ return sig.withArgumentTypes(false, Arrays.asList(type));
+ }).collect(Collectors.toList());
Review Comment:
remove code block, use ImmutableList.of() to generate list
```suggestion
return nested.getSignatures().stream()
.map(sig -> sig.withArgumentTypes(false,
ImmutableList.of(type)))
.collect(ImmutableList.toImmutableList());
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
Review Comment:
final
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionRegistry.java:
##########
@@ -61,6 +62,19 @@ public FunctionBuilder findFunctionBuilder(String name,
Object argument) {
public FunctionBuilder findFunctionBuilder(String name, List<?> arguments)
{
int arity = arguments.size();
List<FunctionBuilder> functionBuilders =
name2Builders.get(name.toLowerCase());
+ if ((functionBuilders == null || functionBuilders.isEmpty())
Review Comment:
`CollectionUtils.isEmpty(functionBuilders)`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java:
##########
@@ -357,8 +360,17 @@ public Expr visitAggregateExpression(AggregateExpression
aggregateExpression, Pl
List<Expression> currentPhaseArguments = child instanceof
AggregateFunction
? child.children()
: aggregateExpression.children();
- return translateAggregateFunction(aggregateExpression.getFunction(),
- currentPhaseArguments, aggFnArguments,
aggregateExpression.getAggregateParam(), context);
+ AggregateFunction function = aggregateExpression.getFunction();
Review Comment:
i don't think u need to change `visitAggregateExpression`, because u have
implement `visitMergeCombinator` the `MergeCombinator` will not come into this
function at all
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectToGlobalAggregate.java:
##########
@@ -82,6 +83,11 @@ public Boolean visitWindow(WindowExpression
windowExpression, Void context) {
return needAggregate;
}
+ @Override
+ public Boolean visitMergeCombinator(MergeCombinator aggregateFunction,
Void context) {
+ return true;
+ }
+
Review Comment:
remove this because MergeCombinator is AggregateFunction, so could be
process by `visitAggregateFunction `
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
+ Preconditions.checkState(nested instanceof AggregateFunction);
+ this.nested = (AggregateFunction) nested;
+ }
+
+ @Override
+ public MergeCombinator withChildren(List<Expression> children) {
+ throw new UnsupportedOperationException("Unimplemented method
'withChildren'");
Review Comment:
why throw exception? we have many place to call withChildren on all
expressions, do not throw exception
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
+ }
+ }
+
+ private BoundFunction buildState(String nestedName, List<? extends Object>
arguments) {
+ return nestedBuilder.build(nestedName, arguments);
+ }
+
+ private BoundFunction buildMergeOrUnion(String nestedName, List<? extends
Object> arguments) {
+ if (arguments.size() != 1 || !(arguments.get(0) instanceof Expression)
+ || !((Expression)
arguments.get(0)).getDataType().isAggStateType()) {
+ String argString = arguments.stream().map(arg -> {
+ if (arg == null) {
+ return "null";
+ } else if (arg instanceof Expression) {
+ return ((Expression) arg).toSql();
+ } else {
+ return arg.toString();
Review Comment:
why arg is not Expression?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java:
##########
@@ -30,22 +30,28 @@
*/
public class AggStateType extends PrimitiveType {
- public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null);
+ public static final AggStateType SYSTEM_DEFAULT = new AggStateType(null,
null, null);
public static final int WIDTH = 16;
private final List<DataType> subTypes;
private final List<Boolean> subTypeNullables;
+ private final String functionName;
- public AggStateType(List<DataType> subTypes, List<Boolean>
subTypeNullables) {
+ public AggStateType(String functionName, List<DataType> subTypes,
List<Boolean> subTypeNullables) {
this.subTypes = subTypes;
this.subTypeNullables = subTypeNullables;
+ this.functionName = functionName;
Review Comment:
```suggestion
this.subTypes =
ImmutableList.copyOf(Objects.requireNonNull(subTypes, "subTypes should not be
null");
this.subTypeNullables = ...;
this.functionName = ...;
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
+ }
+ }
+
+ private BoundFunction buildState(String nestedName, List<? extends Object>
arguments) {
+ return nestedBuilder.build(nestedName, arguments);
+ }
+
+ private BoundFunction buildMergeOrUnion(String nestedName, List<? extends
Object> arguments) {
+ if (arguments.size() != 1 || !(arguments.get(0) instanceof Expression)
+ || !((Expression)
arguments.get(0)).getDataType().isAggStateType()) {
+ String argString = arguments.stream().map(arg -> {
+ if (arg == null) {
+ return "null";
+ } else if (arg instanceof Expression) {
+ return ((Expression) arg).toSql();
+ } else {
+ return arg.toString();
+ }
+ }).collect(Collectors.joining(", ", "(", ")"));
+ throw new IllegalStateException("Can not build AggState nested
function: '" + nestedName + "', expression: "
+ + nestedName + argString);
+ }
+
+ Expression arg = (Expression) arguments.get(0);
+ AggStateType type = (AggStateType) arg.getDataType();
+
+ List<Expression> nestedArgumens = type.getSubTypes().stream().map(t ->
{
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList());
+
+ return nestedBuilder.build(nestedName, nestedArgumens);
+ }
+
+ @Override
+ public BoundFunction build(String name, List<? extends Object> arguments) {
+ String nestedName = getNestedName(name);
+ if (combinatorSuffix.equals(STATE)) {
+ BoundFunction nestedFunction = buildState(nestedName, arguments);
+ return new StateCombinator((List<Expression>) arguments,
nestedFunction);
+ } else {
+ BoundFunction nestedFunction = buildMergeOrUnion(nestedName,
arguments);
+ return new MergeCombinator((List<Expression>) arguments,
nestedFunction);
+ }
+ }
+
+ @Override
+ public String toString() {
+ return combinatorSuffix + "(" + nestedBuilder.toString() + ")";
+ }
+
+ public static boolean isAggStateCombinator(String name) {
+ return name.toLowerCase().endsWith(COMBINATOR_LINKER + STATE)
+ || name.toLowerCase().endsWith(COMBINATOR_LINKER + MERGE)
+ || name.toLowerCase().endsWith(COMBINATOR_LINKER + UNION);
+ }
+
+ public static String getNestedName(String name) {
Review Comment:
should check name contains COMBINATOR_LINKER first, and throw exception if
not contains it
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/StateCombinator.java:
##########
Review Comment:
please add a readme under combinator dir to explain what is xxxCombinator
and how to use them
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java:
##########
@@ -30,22 +30,28 @@
*/
public class AggStateType extends PrimitiveType {
Review Comment:
may be aggStateType should not extends PrimitiveType, since it is not a
primitive type at all
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
Review Comment:
why use `DecimalSamePrecision` trait? this is not used any more
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
+ Preconditions.checkState(nested instanceof AggregateFunction);
+ this.nested = (AggregateFunction) nested;
Review Comment:
all ctors must use ImmutableXXX.copyOf for any collection object. and check
non-null on all non-primitive type attrs
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
+ Preconditions.checkState(nested instanceof AggregateFunction);
Review Comment:
if nested must be `AggregateFunction`, change MergeCombinator parameter to
`AggregateFunction nested` to check it when compile
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/FunctionBuilder.java:
##########
@@ -17,66 +17,19 @@
package org.apache.doris.nereids.trees.expressions.functions;
-import org.apache.doris.common.util.ReflectionUtils;
-import org.apache.doris.nereids.trees.expressions.Expression;
+import avro.shaded.com.google.common.collect.ImmutableList;
Review Comment:
use `com.google.common.collect.ImmutableList`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/MergeCombinator.java:
##########
@@ -0,0 +1,90 @@
+// 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.combinator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import
org.apache.doris.nereids.trees.expressions.functions.AggStateFunctionBuilder;
+import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
+import org.apache.doris.nereids.trees.expressions.functions.ComputeNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.DecimalSamePrecision;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.AggStateType;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * AggState combinator merge
+ */
+public class MergeCombinator extends AggregateFunction
+ implements UnaryExpression, ExplicitlyCastableSignature,
ComputeNullable, DecimalSamePrecision {
+
+ private AggregateFunction nested;
+
+ public MergeCombinator(List<Expression> arguments, BoundFunction nested) {
+ super(nested.getName() + AggStateFunctionBuilder.COMBINATOR_LINKER +
AggStateFunctionBuilder.MERGE, arguments);
+ Preconditions.checkState(nested instanceof AggregateFunction);
+ this.nested = (AggregateFunction) nested;
+ }
+
+ @Override
+ public MergeCombinator withChildren(List<Expression> children) {
+ throw new UnsupportedOperationException("Unimplemented method
'withChildren'");
+ }
+
+ @Override
+ public List<FunctionSignature> getSignatures() {
+ AggStateType type = new AggStateType(nested.getName(),
nested.getArgumentsTypes(),
Review Comment:
we will call getSignatures many times, so u'd better cache the return list
and use the cache when we have cached
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ExpressionVisitor.java:
##########
@@ -433,6 +435,14 @@ public R visitWindowFrame(WindowFrame windowFrame, C
context) {
return visit(windowFrame, context);
}
+ public R visitStateCombinator(StateCombinator combinator, C context) {
Review Comment:
u should not add these two visit here, u should add `visitStateCombinator`
into `ScalarFunctionVistor.java` and add `visitMergeCombinator` into
`AggregateFunctionVisitor.java`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/FunctionBuilder.java:
##########
@@ -17,66 +17,19 @@
package org.apache.doris.nereids.trees.expressions.functions;
-import org.apache.doris.common.util.ReflectionUtils;
-import org.apache.doris.nereids.trees.expressions.Expression;
+import avro.shaded.com.google.common.collect.ImmutableList;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-
-import java.lang.reflect.Array;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.stream.Collectors;
/**
* This class used to resolve a concrete BoundFunction's class and build
BoundFunction by a list of Expressions.
*/
-public class FunctionBuilder {
- public final int arity;
-
- public final boolean isVariableLength;
-
- // Concrete BoundFunction's constructor
- private final Constructor<BoundFunction> builderMethod;
-
- public FunctionBuilder(Constructor<BoundFunction> builderMethod) {
- this.builderMethod = Objects.requireNonNull(builderMethod,
"builderMethod can not be null");
- this.arity = builderMethod.getParameterCount();
- this.isVariableLength = arity > 0 &&
builderMethod.getParameterTypes()[arity - 1].isArray();
- }
-
+public abstract class FunctionBuilder {
/** check whether arguments can apply to the constructor */
public boolean canApply(List<? extends Object> arguments) {
- if (isVariableLength && arity > arguments.size() + 1) {
- return false;
- }
- if (!isVariableLength && arguments.size() != arity) {
- return false;
- }
- for (int i = 0; i < arguments.size(); i++) {
- Class constructorArgumentType = getConstructorArgumentType(i);
- Object argument = arguments.get(i);
- if (!constructorArgumentType.isInstance(argument)) {
- Optional<Class> primitiveType =
ReflectionUtils.getPrimitiveType(argument.getClass());
- if (!primitiveType.isPresent() ||
!constructorArgumentType.isAssignableFrom(primitiveType.get())) {
- return false;
- }
- }
- }
return true;
}
- public Class getConstructorArgumentType(int index) {
- if (isVariableLength && index + 1 >= arity) {
- return builderMethod.getParameterTypes()[arity -
1].getComponentType();
- }
- return builderMethod.getParameterTypes()[index];
- }
-
public BoundFunction build(String name, Object argument) {
Review Comment:
make it final
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/FunctionBuilder.java:
##########
@@ -17,66 +17,19 @@
package org.apache.doris.nereids.trees.expressions.functions;
-import org.apache.doris.common.util.ReflectionUtils;
-import org.apache.doris.nereids.trees.expressions.Expression;
+import avro.shaded.com.google.common.collect.ImmutableList;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-
-import java.lang.reflect.Array;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.stream.Collectors;
/**
* This class used to resolve a concrete BoundFunction's class and build
BoundFunction by a list of Expressions.
*/
-public class FunctionBuilder {
- public final int arity;
-
- public final boolean isVariableLength;
-
- // Concrete BoundFunction's constructor
- private final Constructor<BoundFunction> builderMethod;
-
- public FunctionBuilder(Constructor<BoundFunction> builderMethod) {
- this.builderMethod = Objects.requireNonNull(builderMethod,
"builderMethod can not be null");
- this.arity = builderMethod.getParameterCount();
- this.isVariableLength = arity > 0 &&
builderMethod.getParameterTypes()[arity - 1].isArray();
- }
-
+public abstract class FunctionBuilder {
/** check whether arguments can apply to the constructor */
public boolean canApply(List<? extends Object> arguments) {
Review Comment:
let `canApply` as a abstract interface to make sure all subclass implement
it explicitly
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/FunctionBuilder.java:
##########
@@ -87,68 +40,7 @@ public BoundFunction build(String name, Object argument) {
* @param arguments the function's argument expressions
* @return the concrete bound function instance
*/
- public BoundFunction build(String name, List<? extends Object> arguments) {
- try {
- if (isVariableLength) {
- return
builderMethod.newInstance(toVariableLengthArguments(arguments));
- } else {
- return
builderMethod.newInstance(arguments.stream().toArray(Object[]::new));
- }
- } catch (Throwable t) {
- String argString = arguments.stream()
- .map(arg -> {
- if (arg == null) {
- return "null";
- } else if (arg instanceof Expression) {
- return ((Expression) arg).toSql();
- } else {
- return arg.toString();
- }
- })
- .collect(Collectors.joining(", ", "(", ")"));
- throw new IllegalStateException("Can not build function: '" + name
- + "', expression: " + name + argString + ", " +
t.getCause().getMessage(), t);
- }
- }
-
- private Object[] toVariableLengthArguments(List<? extends Object>
arguments) {
- Object[] constructorArguments = new Object[arity];
+ public abstract BoundFunction build(String name, List<? extends Object>
arguments);
- List<?> nonVarArgs = arguments.subList(0, arity - 1);
- for (int i = 0; i < nonVarArgs.size(); i++) {
- constructorArguments[i] = nonVarArgs.get(i);
- }
-
- List<?> varArgs = arguments.subList(arity - 1, arguments.size());
- Class constructorArgumentType = getConstructorArgumentType(arity);
- Object varArg = Array.newInstance(constructorArgumentType,
varArgs.size());
- for (int i = 0; i < varArgs.size(); i++) {
- Array.set(varArg, i, varArgs.get(i));
- }
- constructorArguments[arity - 1] = varArg;
-
- return constructorArguments;
- }
-
- @Override
- public String toString() {
- return Arrays.stream(builderMethod.getParameterTypes())
- .map(type -> type.getSimpleName())
- .collect(Collectors.joining(", ", "(", ")"));
- }
-
- /**
- * resolve a Concrete boundFunction's class and convert the constructors
to FunctionBuilder
- * @param functionClass a class which is the child class of BoundFunction
and can not be abstract class
- * @return list of FunctionBuilder which contains the constructor
- */
- public static List<FunctionBuilder> resolve(Class<? extends BoundFunction>
functionClass) {
-
Preconditions.checkArgument(!Modifier.isAbstract(functionClass.getModifiers()),
- "Can not resolve bind function which is abstract class: "
- + functionClass.getSimpleName());
- return Arrays.stream(functionClass.getConstructors())
- .filter(constructor ->
Modifier.isPublic(constructor.getModifiers()))
- .map(constructor -> new
FunctionBuilder((Constructor<BoundFunction>) constructor))
- .collect(ImmutableList.toImmutableList());
- }
+ public abstract String toString();
Review Comment:
remove the useless code
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
Review Comment:
```suggestion
return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream()
.map(t -> new SlotReference("mocked", t))
.collect(ImmutableList.toImmutableList()));
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/FunctionBuilder.java:
##########
@@ -17,66 +17,19 @@
package org.apache.doris.nereids.trees.expressions.functions;
-import org.apache.doris.common.util.ReflectionUtils;
-import org.apache.doris.nereids.trees.expressions.Expression;
+import avro.shaded.com.google.common.collect.ImmutableList;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-
-import java.lang.reflect.Array;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.stream.Collectors;
/**
* This class used to resolve a concrete BoundFunction's class and build
BoundFunction by a list of Expressions.
*/
Review Comment:
if u refactor the function builder. please update the java doc comment
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
Review Comment:
check non null
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java:
##########
@@ -430,10 +442,44 @@ public Expr visitIsNull(IsNull isNull,
PlanTranslatorContext context) {
return new IsNullPredicate(isNull.child().accept(this, context),
false, true);
}
+ @Override
+ public Expr visitStateCombinator(StateCombinator combinator,
PlanTranslatorContext context) {
+ List<Expr> arguments = combinator.getArguments().stream().map(arg ->
arg.accept(this, context))
+ .collect(Collectors.toList());
+ return Function.convertToStateCombinator(new FunctionCallExpr(
+ translateAggregateFunction(combinator.getNestedFunction()),
new FunctionParams(false, arguments)));
+ }
+
+ @Override
+ public Expr visitMergeCombinator(MergeCombinator combinator,
PlanTranslatorContext context) {
+ List<Expr> arguments = combinator.getArguments().stream().map(arg ->
arg.accept(this, context))
+ .collect(Collectors.toList());
+ return Function.convertToMergeCombinator(new FunctionCallExpr(
+ translateAggregateFunction(combinator.getNestedFunction()),
new FunctionParams(false, arguments)));
Review Comment:
u should do as follow:
1. construct a new Expression with
combinator.getNestedFunction().withChildren(...)
2. all accept(this, context) on the new Expresssion contructed by step 1
3. do convertToMergeCombinator on the ret value of step 2
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/NormalizeAggregate.java:
##########
@@ -234,5 +235,11 @@ public Void visitAggregateFunction(AggregateFunction
aggregateFunction, List<Agg
context.add(aggregateFunction);
return null;
}
+
+ @Override
+ public Void visitMergeCombinator(MergeCombinator combinator,
List<AggregateFunction> context) {
+ context.add(combinator);
+ return null;
+ }
Review Comment:
remove this, same reason as ProjectToGlobalAggregate rule
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java:
##########
@@ -430,10 +442,44 @@ public Expr visitIsNull(IsNull isNull,
PlanTranslatorContext context) {
return new IsNullPredicate(isNull.child().accept(this, context),
false, true);
}
+ @Override
+ public Expr visitStateCombinator(StateCombinator combinator,
PlanTranslatorContext context) {
+ List<Expr> arguments = combinator.getArguments().stream().map(arg ->
arg.accept(this, context))
+ .collect(Collectors.toList());
+ return Function.convertToStateCombinator(new FunctionCallExpr(
+ translateAggregateFunction(combinator.getNestedFunction()),
new FunctionParams(false, arguments)));
+ }
+
+ @Override
+ public Expr visitMergeCombinator(MergeCombinator combinator,
PlanTranslatorContext context) {
+ List<Expr> arguments = combinator.getArguments().stream().map(arg ->
arg.accept(this, context))
+ .collect(Collectors.toList());
+ return Function.convertToMergeCombinator(new FunctionCallExpr(
+ translateAggregateFunction(combinator.getNestedFunction()),
new FunctionParams(false, arguments)));
+ }
+
+ private org.apache.doris.catalog.AggregateFunction
translateAggregateFunction(AggregateFunction function) {
Review Comment:
why we need to add the new version `translateAggregateFunction`?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
+ }
+ }
+
+ private BoundFunction buildState(String nestedName, List<? extends Object>
arguments) {
+ return nestedBuilder.build(nestedName, arguments);
+ }
+
+ private BoundFunction buildMergeOrUnion(String nestedName, List<? extends
Object> arguments) {
+ if (arguments.size() != 1 || !(arguments.get(0) instanceof Expression)
+ || !((Expression)
arguments.get(0)).getDataType().isAggStateType()) {
+ String argString = arguments.stream().map(arg -> {
+ if (arg == null) {
Review Comment:
why arg could be null?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
+ }
+ }
+
+ private BoundFunction buildState(String nestedName, List<? extends Object>
arguments) {
+ return nestedBuilder.build(nestedName, arguments);
+ }
+
+ private BoundFunction buildMergeOrUnion(String nestedName, List<? extends
Object> arguments) {
+ if (arguments.size() != 1 || !(arguments.get(0) instanceof Expression)
+ || !((Expression)
arguments.get(0)).getDataType().isAggStateType()) {
+ String argString = arguments.stream().map(arg -> {
+ if (arg == null) {
+ return "null";
+ } else if (arg instanceof Expression) {
+ return ((Expression) arg).toSql();
+ } else {
+ return arg.toString();
+ }
+ }).collect(Collectors.joining(", ", "(", ")"));
+ throw new IllegalStateException("Can not build AggState nested
function: '" + nestedName + "', expression: "
+ + nestedName + argString);
+ }
+
+ Expression arg = (Expression) arguments.get(0);
+ AggStateType type = (AggStateType) arg.getDataType();
+
+ List<Expression> nestedArgumens = type.getSubTypes().stream().map(t ->
{
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList());
Review Comment:
```suggestion
List<Expression> nestedArgumens = type.getSubTypes().stream()
.map(t -> new SlotReference("mocked", t))
.collect(ImmutableList.toImmutableList());
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggStateFunctionBuilder.java:
##########
@@ -0,0 +1,126 @@
+// 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;
+
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
+import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
+import org.apache.doris.nereids.types.AggStateType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * This class used to resolve AggState's combinators
+ */
+public class AggStateFunctionBuilder extends FunctionBuilder {
+ public static final String COMBINATOR_LINKER = "_";
+ public static final String STATE = "state";
+ public static final String MERGE = "merge";
+ public static final String UNION = "union";
+
+ private FunctionBuilder nestedBuilder;
+
+ private String combinatorSuffix;
+
+ public AggStateFunctionBuilder(String combinatorSuffix, FunctionBuilder
nestedBuilder) {
+ this.combinatorSuffix = combinatorSuffix;
+ this.nestedBuilder = nestedBuilder;
+ }
+
+ @Override
+ public boolean canApply(List<? extends Object> arguments) {
+ if (combinatorSuffix.equals(STATE)) {
+ return nestedBuilder.canApply(arguments);
+ } else {
+ if (arguments.size() != 1) {
+ return false;
+ }
+ Expression argument = (Expression) arguments.get(0);
+ if (!argument.getDataType().isAggStateType()) {
+ return false;
+ }
+
+ return nestedBuilder.canApply(((AggStateType)
argument.getDataType()).getSubTypes().stream().map(t -> {
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList()));
+ }
+ }
+
+ private BoundFunction buildState(String nestedName, List<? extends Object>
arguments) {
+ return nestedBuilder.build(nestedName, arguments);
+ }
+
+ private BoundFunction buildMergeOrUnion(String nestedName, List<? extends
Object> arguments) {
+ if (arguments.size() != 1 || !(arguments.get(0) instanceof Expression)
+ || !((Expression)
arguments.get(0)).getDataType().isAggStateType()) {
+ String argString = arguments.stream().map(arg -> {
+ if (arg == null) {
+ return "null";
+ } else if (arg instanceof Expression) {
+ return ((Expression) arg).toSql();
+ } else {
+ return arg.toString();
+ }
+ }).collect(Collectors.joining(", ", "(", ")"));
+ throw new IllegalStateException("Can not build AggState nested
function: '" + nestedName + "', expression: "
+ + nestedName + argString);
+ }
+
+ Expression arg = (Expression) arguments.get(0);
+ AggStateType type = (AggStateType) arg.getDataType();
+
+ List<Expression> nestedArgumens = type.getSubTypes().stream().map(t ->
{
+ return new SlotReference("mocked", t);
+ }).collect(Collectors.toList());
+
+ return nestedBuilder.build(nestedName, nestedArgumens);
+ }
+
+ @Override
+ public BoundFunction build(String name, List<? extends Object> arguments) {
+ String nestedName = getNestedName(name);
+ if (combinatorSuffix.equals(STATE)) {
+ BoundFunction nestedFunction = buildState(nestedName, arguments);
+ return new StateCombinator((List<Expression>) arguments,
nestedFunction);
+ } else {
+ BoundFunction nestedFunction = buildMergeOrUnion(nestedName,
arguments);
+ return new MergeCombinator((List<Expression>) arguments,
nestedFunction);
+ }
+ }
+
+ @Override
+ public String toString() {
+ return combinatorSuffix + "(" + nestedBuilder.toString() + ")";
+ }
+
+ public static boolean isAggStateCombinator(String name) {
+ return name.toLowerCase().endsWith(COMBINATOR_LINKER + STATE)
+ || name.toLowerCase().endsWith(COMBINATOR_LINKER + MERGE)
+ || name.toLowerCase().endsWith(COMBINATOR_LINKER + UNION);
+ }
+
+ public static String getNestedName(String name) {
+ return name.substring(0, name.lastIndexOf(COMBINATOR_LINKER));
+ }
+
+ public static String getCombinatorSuffix(String name) {
+ return
name.substring(name.lastIndexOf(AggStateFunctionBuilder.COMBINATOR_LINKER) + 1);
Review Comment:
should check name contains COMBINATOR_LINKER first, and throw exception if
not contains it
--
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
