HappenLee commented on code in PR #66734:
URL: https://github.com/apache/doris/pull/66734#discussion_r3776945662
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java:
##########
@@ -440,11 +441,18 @@ private LogicalPlan
bindGenerate(MatchingContext<LogicalGenerate<Plan>> ctx) {
if (generate.getExpandColumnAlias() != null && i <
generate.getExpandColumnAlias().size()
&&
!CollectionUtils.isEmpty(generate.getExpandColumnAlias().get(i))) {
if (boundSlot.getDataType() instanceof StructType
- && generate.getExpandColumnAlias().get(i).size() > 1) {
+ && (boundGenerator instanceof Stack
Review Comment:
Fixed in 66a3195a9ba. Stack now exposes its logical output-column count, and
the binder uses that width instead of the return value type to decide STRUCT
expansion: width 1 aliases the bound slot as a whole, while width > 1 expands
the synthetic STRUCT carrier and still requires the exact alias count. Added
analyzer coverage for one-field and two-field named_struct values, plus
generated regression cases that return the complete STRUCT values (`{"a":1}`
and `{"a":1, "b":2}`). StackTest passes 9/9 and the stack regression suite
passes.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/generator/Stack.java:
##########
@@ -0,0 +1,150 @@
+// 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.generator;
+
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.rules.expression.rules.FoldConstantRuleOnFE;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
+import org.apache.doris.nereids.trees.expressions.functions.ComputePrecision;
+import org.apache.doris.nereids.trees.expressions.functions.CustomSignature;
+import org.apache.doris.nereids.trees.expressions.literal.IntegerLikeLiteral;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DataType;
+import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.types.NullType;
+import org.apache.doris.nereids.types.StructField;
+import org.apache.doris.nereids.types.StructType;
+import org.apache.doris.nereids.util.ExpressionUtils;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * stack(n, expr1, ..., exprk) separates the expressions into n rows in
row-major order.
+ * Missing values in the last row are padded with nulls.
+ */
+public class Stack extends TableGeneratingFunction implements CustomSignature,
ComputePrecision, AlwaysNullable {
+
+ /** constructor with two or more arguments. */
+ public Stack(Expression numRows, Expression argument, Expression...
otherArguments) {
+ super("stack", ExpressionUtils.mergeArguments(numRows, argument,
otherArguments));
+ }
+
+ /** constructor for withChildren and reuse signature. */
+ private Stack(GeneratorFunctionParams functionParams) {
+ super(functionParams);
+ }
+
+ @Override
+ public Stack withChildren(List<Expression> children) {
+ Preconditions.checkArgument(children.size() >= 2);
+ return new Stack(getFunctionParams(children));
+ }
+
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ getColumnTypes();
+ }
+
+ @Override
+ public FunctionSignature computePrecision(FunctionSignature signature) {
+ return signature;
+ }
+
+ @Override
+ public FunctionSignature searchSignature(List<FunctionSignature>
signatures) {
+ return super.searchSignature(signatures);
+ }
+
+ @Override
+ public FunctionSignature customSignature() {
+ List<DataType> columnTypes = getColumnTypes();
+ List<DataType> argumentTypes = new ArrayList<>(arity());
+ argumentTypes.add(IntegerType.INSTANCE);
+ for (int i = 1; i < arity(); i++) {
+ argumentTypes.add(columnTypes.get((i - 1) % columnTypes.size()));
+ }
+
+ if (columnTypes.size() == 1) {
+ return FunctionSignature.of(columnTypes.get(0), argumentTypes);
+ }
+ ImmutableList.Builder<StructField> fields = ImmutableList.builder();
+ for (int i = 0; i < columnTypes.size(); i++) {
+ fields.add(new StructField("col" + i, columnTypes.get(i), true,
""));
+ }
+ return FunctionSignature.of(new StructType(fields.build()),
argumentTypes);
+ }
+
+ private int getNumRows() {
+ Expression numRowsArgument = getArgument(0);
+ if (!numRowsArgument.isConstant()) {
+ throw new AnalysisException("The first argument of stack must be a
positive constant integer, but got: "
+ + numRowsArgument.toSql());
+ }
+ Expression evaluated = FoldConstantRuleOnFE.evaluate(numRowsArgument,
null);
+ if (!(evaluated instanceof IntegerLikeLiteral)) {
+ throw new AnalysisException("The first argument of stack must be a
positive constant integer, but got: "
+ + numRowsArgument.toSql());
+ }
+ long numRows = ((IntegerLikeLiteral) evaluated).getLongValue();
+ if (numRows <= 0 || numRows > Integer.MAX_VALUE) {
+ throw new AnalysisException("The first argument of stack must be
in (0, " + Integer.MAX_VALUE
+ + "], but got: " + numRows);
+ }
+ return (int) numRows;
+ }
+
+ private List<DataType> getColumnTypes() {
+ int numRows = getNumRows();
+ int numFields = (arity() - 2) / numRows + 1;
+ List<DataType> columnTypes = new ArrayList<>(numFields);
+ for (int columnIndex = 0; columnIndex < numFields; columnIndex++) {
+ DataType referenceType = NullType.INSTANCE;
+ int referenceArgumentIndex = -1;
+ for (int argumentIndex = columnIndex + 1; argumentIndex < arity();
argumentIndex += numFields) {
+ DataType fieldType = getArgument(argumentIndex).getDataType();
+ if (fieldType.isNullType()) {
+ continue;
+ }
+ if (referenceType.isNullType()) {
+ referenceType = fieldType;
+ referenceArgumentIndex = argumentIndex;
+ continue;
+ }
+ if (!referenceType.equals(fieldType)) {
Review Comment:
I checked the Spark implementation and do not think Stack-level widening is
compatible with Spark.
[`Stack.checkInputDataTypes`](https://github.com/apache/spark/blob/2f4bd74e30f5e6b807dd6aa993d1391efc809496/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala#L207-L223)
requires exact equality for every non-NULL value in an output column, while
[`StackTypeCoercion`](https://github.com/apache/spark/blob/2f4bd74e30f5e6b807dd6aa993d1391efc809496/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/StackTypeCoercion.scala#L25-L42)
only coerces `NullType`. The examples pass in Spark because its parser creates
every in-range unsuffixed integer literal as `IntegerType` and every string
literal as `StringType`; Doris instead infers narrow integer types and
`VARCHAR(length)`. Spark would still reject explicitly different types such as
`TINYINT` versus `SMALLINT` with `STACK_COLUMN_DIFF_TYPES`. Computing a common
wider type here would therefor
e be a Doris extension rather than Spark compatibility, so no production
change is made for this thread.
--
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]
