github-actions[bot] commented on code in PR #68499:
URL: https://github.com/apache/doris/pull/68499#discussion_r4435455137
##########
fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditLoader.java:
##########
@@ -46,6 +45,11 @@ public class AuditLoader extends Plugin implements
AuditPlugin {
public static final String AUDIT_LOG_TABLE = "audit_log";
+ /**
+ * How long the load worker sleeps between queue polls (millis).
+ */
Review Comment:
[P2] Keep idle audit workers wakeable by queue arrival. After an empty
`transferNextEvent()`, this unconditional five-second sleep cannot be
interrupted by `exec()`, so an event arriving immediately afterward waits up to
five seconds before `loadIfNecessary` runs. That violates a configured
`auditPluginMaxBatchInternalSec` below five seconds and regresses audit-log
freshness versus the prior timed `BlockingQueue.poll`, which woke as soon as an
event was enqueued. Use the timed blocking poll or signal the worker on enqueue.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/spm/placeholder/SPMPlaceholderBuilder.java:
##########
@@ -0,0 +1,1541 @@
+// 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.catalog.FunctionSignature;
+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.BoundFunction;
+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 org.apache.doris.nereids.types.coercion.FollowToArgumentType;
+
+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.Locale;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * 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 {
+
+ /**
+ * The ARRAY scalar functions that declare {@code retArgType(0)}: each
one's result IS its
+ * first argument's array type, so an all-NULL input makes the whole
result an all-NULL array.
+ *
+ * <p>They are exactly the wrappers a projected all-NULL container can sit
UNDER while the
+ * enclosing call is still consumed as a Boolean operand, e.g.
+ * {@code k > 0 OR element_at(array_reverse_sort(array(NULL)), 1)}: the
wrapper hands
+ * {@code array(NULL)}'s NullType element type to {@code element_at}, so
the marker has to be
+ * repaired at the WRAPPER's argument position. Matching only {@code
array_sort} /
+ * {@code array_distinct} left every other member of the family
unrecognized (round-87
+ * review).
+ *
+ * <p>Matched by NAME because the capture runs on the PARSE tree, where
the call is still an
+ * unbound {@link Function} and no signature is resolved yet; a BOUND node
is classified from
+ * its {@code retArgType(0)} signature instead (see followsFirstArgument),
so a function added
+ * to the family is recognized without touching this list. {@code
array_concat} is NOT here:
+ * it derives its element type from EVERY part and is handled separately
+ * (isArrayConcatCall).
+ */
+ private static final Set<String> TYPE_PRESERVING_ARRAY_CALLS = Set.of(
+ "array_apply", "array_compact", "array_difference",
"array_distinct",
+ "array_except", "array_except_all", "array_filter", "array_first",
+ "array_intersect", "array_last", "array_popback", "array_popfront",
+ "array_pushback", "array_pushfront", "array_remove",
"array_reverse_sort",
+ "array_shuffle", "array_slice", "array_sort", "array_sortby",
"array_union",
+ "reverse", "trim_array");
+
+ /** The MAP counterpart of {@link #TYPE_PRESERVING_ARRAY_CALLS} (same
{@code retArgType(0)}
+ * declaration: {@code map_filter} returns its first argument's map type).
*/
+ private static final Set<String> TYPE_PRESERVING_MAP_CALLS =
Set.of("map_filter");
+
+ /** 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;
+
+ /**
+ * Depth of a CONTAINER CONSTRUCTOR (`array(...)`) whose ELEMENTS are the
markers that carry
+ * the container's element type (see markerPayload): an all-NULL element
list is what makes
+ * the constructor NullType, so its NULL elements are parameterized
BOOLEAN while the
+ * constructor itself sits in a container position of a NullType-producing
chain.
+ */
+ private int containerElementDepth;
+
+ /**
+ * Depth of a MAP CONSTRUCTOR (`map(...)`) whose VALUE markers carry the
map's value type (see
+ * markerPayload): only the ODD child positions are values - the keys feed
the map's KEY type
+ * and keep their own typing, exactly like the map-literal repair does.
+ */
+ private int mapValueElementDepth;
+
+ /**
+ * 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 (isTypePreservingMapCall(expr)) {
+ // A TYPE-PRESERVING MAP WRAPPER (`map_filter`) forwards its
INPUT's MAP value type,
+ // so its result is MAP<..., NULL_TYPE> exactly when the input is
(round-91 review):
+ // `element_at(map_filter(map('x', NULL), (m_k, m_v) -> TRUE),
'x')` stayed a NullType
+ // producer, `visitOr` replaced the whole ElementAt operand with a
Boolean NULL and
+ // erased its markers, and the `map('x', TRUE)` variant then
replayed NULL instead of
+ // TRUE. Recursing through the wrapper keeps the family in ONE
classifier.
+ List<Expression> args = expr.children();
+ return !args.isEmpty() && isAllNullMap(args.get(0));
+ }
+ 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) {
Review Comment:
[P1] Preserve placeholders through FollowToAnyDataType array producers.
`array_repeat(NULL, 1)` resolves to `ARRAY<NULL_TYPE>`, but `isAllNullArray`
does not classify this `FollowToAnyDataType(0)` producer. In `k > 0 OR
element_at(array_repeat(NULL, 1), 1)`, Boolean analysis replaces the NullType
element-at operand with a typed NULL and removes its marker; the value-variant
`array_repeat(TRUE, 1)` can still match the value-free digest, but frozen
replay has no marker to substitute and returns NULL when the caller returns
TRUE. Extend the container repair to this signature family (or reject the
frozen shape) and add a NULL-to-TRUE replay case.
--
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]
