github-actions[bot] commented on code in PR #68499: URL: https://github.com/apache/doris/pull/68499#discussion_r4414478183
########## fe/fe-core/src/main/java/org/apache/doris/nereids/spm/placeholder/SPMPlaceholderBuilder.java: ########## @@ -0,0 +1,1369 @@ +// 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.spm.placeholder; + +import org.apache.doris.nereids.spm.SPMPlanTreeSupport; +import org.apache.doris.nereids.trees.expressions.And; +import org.apache.doris.nereids.trees.expressions.CaseWhen; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.InPredicate; +import org.apache.doris.nereids.trees.expressions.InSubquery; +import org.apache.doris.nereids.trees.expressions.Like; +import org.apache.doris.nereids.trees.expressions.Not; +import org.apache.doris.nereids.trees.expressions.Or; +import org.apache.doris.nereids.trees.expressions.SubqueryExpr; +import org.apache.doris.nereids.trees.expressions.WhenClause; +import org.apache.doris.nereids.trees.expressions.functions.Function; +import org.apache.doris.nereids.trees.expressions.functions.scalar.ElementAt; +import org.apache.doris.nereids.trees.expressions.literal.ArrayLiteral; +import org.apache.doris.nereids.trees.expressions.literal.Literal; +import org.apache.doris.nereids.trees.expressions.literal.MapLiteral; +import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; +import org.apache.doris.nereids.types.ArrayType; +import org.apache.doris.nereids.types.BooleanType; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.MapType; + +import com.google.common.collect.ImmutableList; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.List; +import java.util.Map; + +/** + * SPMPlaceholderBuilder + * + * Replaces the literal constants in a query with + * SPM placeholder functions so that one baseline can match structurally identical + * queries. Core replacement rules: + * + * - Literal -> SpmConstVar (scalar placeholder with an auto-increment id) + * - "IN (constant list)" -> InPredicate(expr, [SpmConstList(id, ...)]) (list placeholder) + * + * The auto-increment id guarantees that placeholders in the same position of bindSql and + * planSql share the same id (in path A auto-capture both come from the same already + * parameterized tree, so the ids naturally match). + * + * Placeholder deduplication follows: a literal + * reuses an existing placeholder id only when the literal value (Step 1), its parent + * expression structure (Step 2, PlaceholderExpr.matches) AND its position inside that + * parent (child index) are equal. This is important for matching correctness: with + * value-only dedup, "a = 1 AND b = 1" would assign one id to both literals and a user + * query "a = 5 AND b = 7" could never match; without the child index, the two operands + * of "1 + 1" would share one id and a similar query "2 + 1" could never match either. + */ +public class SPMPlaceholderBuilder extends ExpressionVisitor<Expression, Expression> + implements SPMPlanTreeSupport.ExprTransform { + + /** Auto-increment placeholder id counter (starts at 1). */ + private long nextId = 1; + + /** Placeholder record list. */ + private final List<PlaceholderExpr> placeholderExprs = new ArrayList<>(); + + /** + * Optional GROUP BY mirror lookup, installed by SPMPlanTreeSupport.TreeTransformer for + * one clause window (see setGroupByMirrorLookup): maps an expression's + * pre-parameterization text to the ALREADY-transformed GROUP BY expression that must + * be reused - also for NESTED subexpressions (e.g. the substr(...) inside + * "HAVING substr(...) = 'x'"), because the analyzer resolves such a clause + * expression-wise and demands literal expression equality with the GROUP BY under + * ONLY_FULL_GROUP_BY. Null outside the window. + */ + private java.util.function.Function<String, Expression> groupByMirrorLookup; + + /** + * DFS stack of child positions: the top is the index of the expression currently + * being visited inside its direct parent (empty = parent-less root). Two literals + * under the same parent (e.g. the operands of "1 + 1") are different positions and + * must never reuse one placeholder id. + */ + private final Deque<Integer> childIndexStack = new ArrayDeque<>(); + + /** + * Query-block identity: incremented when entering a nested query block (the plan of + * a subquery expression). Two literals in DIFFERENT blocks never share a placeholder + * id even when value / parent / position coincide: in + * SELECT * FROM t WHERE a=1 AND EXISTS (SELECT 1 FROM u WHERE a=1) the two a=1 + * literals used to reuse one id, and a user query with outer a=2 / inner a=3 was + * rejected (one id must resolve to ONE value), so the baseline could never hit. + * The counter restarts per tree (see startNewTree): bind and plan trees traverse the + * same structure in the same order, so corresponding blocks keep corresponding + * numbers and the cross-tree id alignment is preserved. + */ + private long nextBlockId = 1; + private long currentBlockId = 0; + + /** + * Enclosing query-block ids (see enterQueryBlock()): a nested block's id is + * restored on exit, so SIBLING derived tables / CTE bodies get distinct ids while the + * enclosing block numbering resumes - the same discipline the subquery-expression + * blocks already followed. + */ + private final Deque<Long> blockIdStack = new ArrayDeque<>(); + + /** + * Projection ITEM positions of the SELECT-list slots currently being visited (see + * enterProjectionItem(int)): SELECT 1 AS x, 1 AS y must give its two + * literals different placeholder ids although value, parent structure and child + * position coincide (see PlaceholderExpr#matches). + */ + private final Deque<Integer> projectItemStack = new ArrayDeque<>(); + + /** + * Ordinal of the expression ROOT currently being transformed, counted per query block in + * walk order (see beginExpressionRoot): two equal literals in DIFFERENT roots of one + * block (the ON clauses of two joins, two conjuncts of one filter, ...) used to share one + * placeholder id although they are independent slots - a query that varies them + * separately (1/1 -> 2/3) was then rejected by recordPlaceholderValue and the saved + * baseline could never be used (the reviewer's round-58 finding). The counting restarts + * per tree run AND per block, so the bind and plan trees of one baseline traverse the + * same structure in the same order and corresponding roots keep corresponding ordinals. + */ + private long nextRootOrdinal = 0; + private long currentRootOrdinal = -1; + + /** + * Depth of an all-NULL CONDITIONAL (CASE / IF / COALESCE) currently visited in a Boolean + * context (see markerPayload): such a node has NullType and the analyzer replaces the whole + * operand with a typed constant, so its branch NULLs are parameterized BOOLEAN. + */ + private int booleanConditionalDepth; + + /** + * Whether a bare integer GROUP BY ordinal references a SELECT item that carries a + * placeholder (see SPMPlanTreeSupport#visitLogicalAggregate). The ordinal itself stays + * CONCRETE, but the analyzer resolves it to that item's (parameterized) expression, and + * the frozen rendering of a placeholder group key is NOT faithful: the capture optimizer + * rewrote `GROUP BY <placeholder>` into a child column, so a frozen replay of + * `SELECT 1 AS c, count(*) ... GROUP BY 1` grouped by `k` while the user's own query + * grouped by the constant. The CREATE keeps the user planSql then and replays the + * parameterized tree, whose analysis resolves the ordinal in the caller's own query. + */ + private boolean parameterizedGroupByOrdinalTarget; + + /** The enclosing block's root-ordinal counter, restored by exitQueryBlock(). */ + private final Deque<Long> rootOrdinalStack = new ArrayDeque<>(); + + /** The enclosing root's ordinal, restored by exitQueryBlock() (see enterQueryBlock). */ + private final Deque<Long> currentRootOrdinalStack = new ArrayDeque<>(); + + /** Installs / clears the GROUP BY mirror lookup window (null = disabled). */ + public void setGroupByMirrorLookup(java.util.function.Function<String, Expression> lookup) { + this.groupByMirrorLookup = lookup; + } + + /** See parameterizedGroupByOrdinalTarget. */ + public boolean hasParameterizedGroupByOrdinalTarget() { + return parameterizedGroupByOrdinalTarget; + } + + /** Marks one bare integer GROUP BY ordinal whose SELECT item carries placeholders. */ + public void noteParameterizedGroupByOrdinalTarget() { + this.parameterizedGroupByOrdinalTarget = true; + } + + /** + * Parameterizes a list of expressions with this single shared builder (entry point). + * + * Every expression is parameterized in order with the same builder, so placeholder + * ids stay globally unique and position-aligned across all query blocks (the topmost + * block's literals get the lowest ids, then derived tables / CTE bodies / nested + * blocks, in DFS order). This is what keeps the bind expressions of a multi-block + * baseline matched against the user query block by block. + * + * @param exprs the raw expressions to parameterize (one per query block) + * @return the parameterized expressions (containing SpmConstVar / SpmConstList) + */ + public List<Expression> parameterizeExpressions(List<Expression> exprs) { + List<Expression> parameterized = new ArrayList<>(exprs.size()); + for (Expression expr : exprs) { + // each element is one query block of the same tree: its own block scope + startNewTree(); + beginExpressionRoot(); + parameterized.add(expr.accept(this, null)); + } + return parameterized; + } + + /** Starts one expression ROOT of the current block (see nextRootOrdinal). */ + private void beginExpressionRoot() { + currentRootOrdinal = nextRootOrdinal++; + } + + /** Restarts the per-tree query-block numbering (a tree's root block is 0). */ + public void startNewTree() { + nextBlockId = 1; + currentBlockId = 0; + blockIdStack.clear(); + projectItemStack.clear(); + // one tree = one root-ordinal run: corresponding roots of the bind tree and the + // (separately parsed) plan tree keep corresponding ordinals + nextRootOrdinal = 0; + currentRootOrdinal = -1; + rootOrdinalStack.clear(); + currentRootOrdinalStack.clear(); + } + + /** + * The whole-tree parameterization transform: the builder IS an + * SPMPlanTreeSupport.ExprTransform, so the plan walk can hand it the query + * BLOCK and projection ITEM scopes of the expressions it visits (see + * SPMPlanTreeSupport.ExprTransform#enterQueryBlock()). Passing a bare lambda + * instead would lose those scopes and merge independent literals into one placeholder + * id. + */ + @Override + public Expression apply(Expression expr) { + beginExpressionRoot(); + return expr.accept(this, null); + } + + /** + * Transform of a SUBQUERY expression's own plan tree: every expression the walk hands + * over is a block ROOT (the child-position sentinel -1), and the query-block / + * projection-item scope hooks are forwarded to the enclosing builder so nested + * derived tables keep their own block ids. + */ + private final class SubqueryBlockTransform implements SPMPlanTreeSupport.ExprTransform { + @Override + public Expression apply(Expression expr) { + beginExpressionRoot(); + childIndexStack.push(-1); + try { + return expr.accept(SPMPlaceholderBuilder.this, null); + } finally { + childIndexStack.pop(); + } + } + + @Override + public void enterQueryBlock() { + SPMPlaceholderBuilder.this.enterQueryBlock(); + } + + @Override + public void exitQueryBlock() { + SPMPlaceholderBuilder.this.exitQueryBlock(); + } + + @Override + public void enterProjectionItem(int index) { + SPMPlaceholderBuilder.this.enterProjectionItem(index); + } + + @Override + public void exitProjectionItem() { + SPMPlaceholderBuilder.this.exitProjectionItem(); + } + } + + /** + * Enters a nested query block (see + * org.apache.doris.nereids.spm.SPMPlanTreeSupport.ExprTransform#enterQueryBlock()): + * the block gets its own monotonic id, the enclosing one is restored by + * exitQueryBlock(). + */ + @Override + public void enterQueryBlock() { + blockIdStack.push(currentBlockId); + currentBlockId = nextBlockId++; + // the root-ordinal counting is per BLOCK as well: a nested block's roots must not + // shift the enclosing block's ordinals (and vice versa), and the enclosing counters + // resume on exit + rootOrdinalStack.push(nextRootOrdinal); + nextRootOrdinal = 0; + currentRootOrdinalStack.push(currentRootOrdinal); + currentRootOrdinal = -1; + } + + /** Leaves the current nested query block and restores the enclosing block id. */ + @Override + public void exitQueryBlock() { + if (!blockIdStack.isEmpty()) { + currentBlockId = blockIdStack.pop(); + } + if (!rootOrdinalStack.isEmpty()) { + nextRootOrdinal = rootOrdinalStack.pop(); + } + if (!currentRootOrdinalStack.isEmpty()) { + currentRootOrdinal = currentRootOrdinalStack.pop(); + } + } + + /** + * Enters one projection item of a projection-like list: every literal of its subtree + * carries this position, so two equally shaped literals in different items (or in a + * projection and in a filter) never share one placeholder id. + */ + @Override + public void enterProjectionItem(int index) { + projectItemStack.push(index); + } + + /** Leaves the current projection item. */ + @Override + public void exitProjectionItem() { + if (!projectItemStack.isEmpty()) { + projectItemStack.pop(); + } + } + + /** Position of the projection item currently being visited; -1 outside a SELECT list. */ + private int currentProjectItem() { + Integer top = projectItemStack.peek(); + return top == null ? -1 : top; + } + + public List<PlaceholderExpr> getPlaceholderExprs() { + return placeholderExprs; + } + + /** Position of the expression currently being visited inside its direct parent; -1 at a root. */ + private int currentChildIndex() { + Integer top = childIndexStack.peek(); + return top == null ? -1 : top; + } + + /** + * The occurrence path of the literal currently being visited: the child positions from the + * transformed EXPRESSION ROOT down to the literal, rendered as "0/1/..." (empty for a + * root literal). Two equal literals can sit under structurally EQUAL parents at + * different places of one root ("a = 1 AND a = 1": the parents are the two And + * children), and only the path tells those independent slots apart - see + * PlaceholderExpr#occurrencePath. + */ + private String currentOccurrencePath() { + if (childIndexStack.isEmpty()) { + return ""; + } + Object[] positions = childIndexStack.toArray(); + StringBuilder sb = new StringBuilder(); + for (int i = positions.length - 1; i >= 0; i--) { + if (sb.length() > 0) { + sb.append('/'); + } + sb.append(positions[i]); + } + return sb.toString(); + } + + // ==================== literal -> scalar placeholder ==================== + + /** + * Whether the expression's TYPE is NullType. + * + * The type is derived structurally because this builder walks the UNBOUND plan, where + * {@code getDataType()} throws. A node produces NullType when EVERY value it can yield is + * NULL - a NULL literal, a CASE whose every result does, an IF whose two value arguments do, + * a COALESCE / IFNULL / NVL / NULLIF / GREATEST / LEAST all of whose arguments do. Collected as + * a Boolean operand (directly, or through a projected slot) such an expression is REPLACED by + * the analyzer with a typed constant NULL, which deletes the placeholders inside it. + * + * The test is RECURSIVE on purpose (round-78 #5): a conditional nested in the RESULT of + * another conditional (`k > 0 OR (CASE WHEN k = 0 THEN ifnull(NULL, NULL) ELSE NULL END)`) + * yields NullType although its result is not a NULL LITERAL, so a literal-only test left its + * inner markers NullType and the consumer erased them - a TRUE variant then replayed as NULL. + * The same recursion covers the custom-signature producers: {@code GreatestLeast#customSignature} + * returns the arguments' common type, i.e. NullType for {@code greatest(NULL, NULL)} (round-78 + * #1, LEAST shares the path). + * + * @param expr the expression to classify + * @return whether every value the expression can yield is NULL + */ + private static boolean producesNullType(Expression expr) { + if (expr == null) { + return false; + } + if (expr.isNullLiteral()) { + return true; + } + List<Expression> children = expr.children(); + if (expr instanceof CaseWhen) { + CaseWhen caseWhen = (CaseWhen) expr; + for (WhenClause clause : caseWhen.getWhenClauses()) { + if (!producesNullType(clause.getResult())) { + return false; + } + } + return !caseWhen.getDefaultValue().isPresent() + || producesNullType(caseWhen.getDefaultValue().get()); + } + if (isIfConditional(expr)) { + return children.size() == 3 && producesNullType(children.get(1)) + && producesNullType(children.get(2)); + } + if (isIfNullConditional(expr) || isNullIfConditional(expr) + || isGreatestLeastConditional(expr) || isCoalesceConditional(expr)) { + // every argument contributes a value the result may equal: all-NULL arguments make + // the whole call NullType (IFNULL / NVL / NULLIF / GREATEST / LEAST / COALESCE) + if (children.isEmpty()) { + return false; + } + for (Expression child : children) { + if (!producesNullType(child)) { + return false; + } + } + return true; + } + if (isElementAtCall(expr)) { + // `element_at(array(NULL), 1)` takes its type from the ARRAY's element type and + // `element_at(map('x', NULL), 'x')` from the MAP's VALUE type (CreateMap derives + // MAP<STRING, NULL_TYPE>), so an all-NULL container makes the WHOLE call NullType: a + // Boolean consumer replaced the two markers of + // `k > 0 OR element_at(array(NULL), 1)` with a typed constant and the + // `element_at(array(TRUE), 1)` variant had nothing to substitute (round-79 #3), and + // the MAP spelling lost the value marker the same way (round-80 #1). + return !children.isEmpty() && isAllNullContainer(children.get(0)); + } + if (isArrayReducerCall(expr)) { + // The SAME producer classification, for the array REDUCERS (round-81 P1): + // ARRAY_MIN / ARRAY_MAX return `FollowToAnyDataType(0)`, i.e. the INPUT array's + // element type, so an all-NULL input makes the whole call NullType - binding then + // replaced `k > 0 OR array_min(array(NULL))` with `k > ph OR NULL` and the + // `array_min(array(TRUE))` caller got NULL instead of TRUE. Keeping the family in + // ONE classifier is what lets the marker be retyped before the Boolean consumer + // drops it - the same defect the bare element_at trigger had, so it is carried here + // instead of in a per-function inline. + return !children.isEmpty() && isAllNullArray(children.get(0)); + } + return false; + } + + /** + * Whether the expression is a container whose ELEMENT type is NullType (see producesNullType): + * an all-NULL array or an all-NULL-valued map. Both overloads of `element_at` return the + * container's element / value type, so either one makes the call a NullType producer. + * + * @param expr the expression to classify + * @return whether it is an all-NULL array or map value + */ + private static boolean isAllNullContainer(Expression expr) { + return isAllNullArray(expr) || isAllNullMap(expr); + } + + /** + * Whether the expression builds a MAP whose every VALUE produces NullType (see + * producesNullType). + * + * The children of a map constructor alternate KEY, VALUE, so only the odd positions feed the + * map's value type - and with it what `element_at` returns. Both spellings are accepted: the + * parsed `map(...)` call and the bound {@code CreateMap}, which is a {@link Function} named + * {@code map}. A MAP LITERAL ({@code MapLiteral}) is a leaf without children and is therefore + * not covered - its entries are never parameterized either. + * + * @param expr the expression to classify + * @return whether it is an all-NULL-valued map value + */ + private static boolean isAllNullMap(Expression expr) { + if (expr == null) { + return false; + } + if (expr instanceof MapLiteral) { + // A map LITERAL is a LEAF: its entries live in getValue(), not in children() + // (round-84 #1). visitLiteral parameterizes the whole literal as ONE marker, and + // without this branch `element_at({'x': NULL}, 'x')` stayed a NullType producer + // candidate, the Boolean consumer replaced the whole call and the marker with it, + // and the `{'x': TRUE}` variant had no id left to substitute. + Map<Literal, Literal> entries = ((MapLiteral) expr).getValue(); + if (entries.isEmpty()) { + // the same argument as for the empty ARRAY literal: without an entry there is no + // value type to inherit, so the empty map is NullType-valued as well (round-85) + return true; + } + for (Literal value : entries.values()) { + if (!producesNullType(value)) { + return false; + } + } + return true; + } + if (!isMapConstructor(expr)) { + return false; + } + List<Expression> pairs = expr.children(); + if (pairs.size() < 2) { + return false; + } + for (int i = 1; i < pairs.size(); i += 2) { + if (!producesNullType(pairs.get(i))) { + return false; + } + } + return true; + } + + /** + * Whether the expression is an all-NULL array (see producesNullType). + * + * An array LITERAL whose every element produces NullType has type ARRAY<NULL_TYPE>, so a + * consumer that reads an ELEMENT (`element_at`) is NullType as well. Both spellings are + * accepted: the parsed {@link ArrayLiteral} and the unbound `array(...)` call, which is only + * a {@link Function} named {@code array} before binding. + * + * An ARRAY_CONCAT WRAPPER is an all-NULL array exactly when every concatenated part is one + * (round-80): the function derives its element type from its inputs, so concatenating only + * all-NULL arrays keeps ARRAY<NULL_TYPE> - and the wrapper was NOT recognized, so + * `k > 0 OR element_at(array_concat(array(NULL), array(NULL)), 1)` kept an ARRAY<NULL_TYPE> + * marker, a Boolean consumer erased the element_at call and its markers, and the + * `array(TRUE), array(TRUE)` variant had nothing left to substitute (the frozen replay then + * returned NULL for a row the caller returns TRUE for). + * + * A TYPE-PRESERVING ARRAY TRANSFORMATION ({@code array_sort} / {@code array_distinct}) wraps + * the same way (round-81): both declare {@code retArgType(0)}, so the result is + * ARRAY<NULL_TYPE> exactly when their INPUT array is - and the unrecognized wrapper left + * the element_at call NullType, which made the Boolean consumer drop it together with its + * markers (`k > 0 OR element_at(array_sort(array(NULL)), 1)` then replayed NULL for k = -1 + * although the {@code array_sort(array(TRUE))} caller returns TRUE). + * + * MAP_VALUES converts the MAP-VALUE type into an array of it (round-83 #3): + * `map_values(map('x', NULL))` is ARRAY<NULL_TYPE>, so the same repair applies to + * `element_at(map_values(map('x', NULL)), 1)` - the wrapper was unrecognized, the value marker + * stayed NullType and `visitOr` erased the whole call. + * + * @param expr the expression to classify + * @return whether it is an all-NULL array value + */ + private static boolean isAllNullArray(Expression expr) { + if (expr == null) { + return false; + } + if (expr.isNullLiteral()) { + // a NULL literal is untyped ARRAY<NULL_TYPE> in an array position + return true; + } + if (isArrayConcatCall(expr)) { + List<Expression> parts = expr.children(); + if (parts.isEmpty()) { + return false; + } + for (Expression part : parts) { + if (!isAllNullArray(part)) { + return false; + } + } + return true; + } + if (isMapValuesCall(expr)) { + // MAP_VALUES follows its MAP's value type (see the doc above) + List<Expression> parts = expr.children(); + return parts.size() == 1 && isAllNullMap(parts.get(0)); + } + if (isTypePreservingArrayCall(expr)) { + // only the FIRST child is the array whose element type the result inherits: the + // comparator of `array_sort(array, lambda)` contributes no type of its own + List<Expression> parts = expr.children(); + return !parts.isEmpty() && isAllNullArray(parts.get(0)); + } + if (!isArrayConstructor(expr)) { + return false; + } + // a bracket array LITERAL is a LEAF: its elements live in getValue(), not in children() + List<? extends Expression> elements = expr instanceof ArrayLiteral + ? ((ArrayLiteral) expr).getValue() : expr.children(); + if (elements.isEmpty()) { + // An EMPTY array literal carries no element to take a type from, so its element type + // is NullType as well (round-85): without this branch `element_at([], 1)` was NOT + // classified as a NullType producer, the Boolean context never reached the container + // marker, and the payload kept ARRAY<NULL_TYPE> - the consumer then replaced the + // whole operand together with its markers and the `element_at([TRUE], 1)` caller had + // nothing to substitute. + return true; + } + for (Expression element : elements) { + if (!producesNullType(element)) { + return false; + } + } + return true; + } + + /** + * Whether the expression is an {@code array_concat} call (see isAllNullArray). + * + * Matched by NAME through {@link Function}, which covers both the unbound `array_concat(...)` + * call and the bound {@code ArrayConcat} node (its function name is {@code array_concat}). + * + * @param expr the expression to classify + * @return whether it is an array_concat call + */ + private static boolean isArrayConcatCall(Expression expr) { + return expr instanceof Function + && "array_concat".equalsIgnoreCase(((Function) expr).getName()); + } + + /** + * Whether the expression is an {@code array_sort} / {@code array_distinct} call (see + * isAllNullArray). + * + * Both declare {@code retArgType(0)}, i.e. the result's element type is the INPUT array's, so + * an all-NULL input makes the result an all-NULL array. Matched by NAME through + * {@link Function}, which covers the unbound calls and the bound {@code ArraySort} / + * {@code ArrayDistinct} nodes (their function names are {@code array_sort} / + * {@code array_distinct}). + * + * @param expr the expression to classify + * @return whether it is a type-preserving array transformation + */ + private static boolean isTypePreservingArrayCall(Expression expr) { Review Comment: [P1] Classify every type-preserving array wrapper before Boolean marker repair. `isTypePreservingArrayCall` only accepts `array_sort` and `array_distinct`, but `array_reverse_sort` (and the retArgType(0) slice/union family) forwards the input element type. A capture such as `k > 0 OR element_at(array_reverse_sort(array(NULL)), 1)` is therefore analyzed as a NullType operand and loses its placeholder; the matching `array(TRUE)` query extracts TRUE but replay keeps NULL for `k <= 0`. Generalize this check from the function signatures or reject the shape, and add a replay case for these wrappers. ########## fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java: ########## @@ -91,6 +93,7 @@ import org.apache.doris.nereids.trees.plans.commands.DeleteFromCommand; import org.apache.doris.nereids.trees.plans.commands.DeleteFromUsingCommand; import org.apache.doris.nereids.trees.plans.commands.EmptyCommand; Review Comment: [P2] Route query EXPLAIN through the forwarding decision. `isForwardableQuery()` recognizes `ExplainCommand`, but command dispatch invokes `isForwardToMaster()` only for `Redirect` commands and `ExplainCommand` still implements `NoForward`, so this new helper never affects EXPLAIN execution. On an observer with forced forwarding, EXPLAIN can report a stale local baseline while the corresponding SELECT uses the master's newer one. Apply the same forwarding path before `ExplainCommand.run` and cover observer/forced-forward EXPLAIN plus SELECT. -- 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]
