github-actions[bot] commented on code in PR #66734: URL: https://github.com/apache/doris/pull/66734#discussion_r3776615412
########## 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: [P1] Widen compatible values within each Stack column This exact equality check runs before generic signature coercion. Doris deliberately infers ordinary literals narrowly, so these valid calls are rejected: ```text stack(2, 1, 128) // TINYINT vs SMALLINT stack(2, 'a', 'long') // VARCHAR(1) vs VARCHAR(4) ``` Reduced column grouping for the first call is: ```text Stack(rows=2, width=1) output col0 <- [1:TINYINT, 128:SMALLINT] ``` Both pairs have a lossless common Doris type, but `getColumnTypes()` throws before `customSignature()` can request casts. Merely retaining the first type would be lossy for `128`. Please compute a common wider type across the non-NULL values of each output column, use it for both the return field and corresponding argument types, and reject only when no compatible type exists. Add integer-width and VARCHAR-length regressions; this is the compatibility contract called out by the issue and is distinct from the live row-count and alias threads. ########## 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: [P1] Preserve STRUCT values when Stack has one output column This condition conflates Stack's synthetic multi-column carrier with a value that is itself a STRUCT. For example: ```sql select c from (select 1) t lateral view stack(2, named_struct('a', 1, 'b', 2), named_struct('a', 3, 'b', 4)) s as c; ``` The reduced tree inferred from this binding path is: ```text LogicalGenerate(stack(...) -> #value: STRUCT<a INT,b INT>, expandAlias=[c]) <child> ``` `Stack.customSignature()` derives logical width one and returns that STRUCT as the single value type; BE's `_num_fields == 1` path likewise emits each whole struct. Here `instanceof StructType && boundGenerator instanceof Stack` instead treats fields `a,b` as two generated columns, so binding rejects one alias for two fields. A one-field STRUCT is silently flattened through `ElementAt`. Please decide expansion from Stack's derived output width rather than from the value type: width one must alias the bound slot as a whole. Add regressions for both one-field and two-field `named_struct` values. This is distinct from the live thread about too few aliases for a true multi-column Stack result. -- 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]
