github-actions[bot] commented on code in PR #68019:
URL: https://github.com/apache/doris/pull/68019#discussion_r4033668336
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/LazyMaterializeTopN.java:
##########
@@ -292,6 +298,98 @@ private void collectProjectExprInputSlots(Plan plan,
Set<Slot> requiredMateriali
}
}
+ /**
+ * Keep the columns consumed below the TopN materialized during the scan,
resolved through identity
+ * aliases.
+ *
+ * <p>{@link MaterializeProbeVisitor} only protects the slot it is
tracing: a slot consumed by an
+ * operator on the way from the TopN down to the relation stops the probe,
but the columns an
+ * identity alias reads are never resolved. For
+ *
+ * <pre>
+ * OuterTopN(order by z)
+ * InnerTopN(order by x)
+ * Project(lazy_col AS x, lazy_col AS y, other_col AS z)
+ * OlapScan
+ * </pre>
+ *
+ * probing the outer output {@code y} resolves to the base column {@code
lazy_col}, so {@code lazy_col}
+ * is classified lazy and {@link LazySlotPruning} removes it from the
scan, while {@code lazy_col AS x}
+ * below the outer TopN is still read by the inner TopN. The plan then
references a slot its child no
+ * longer produces and the final {@link Validator} rejects it. The same
happens when an identity alias
+ * is consumed by a filter, a join condition or any other operator that
stays below the TopN.
+ *
+ * <p>Therefore every slot consumed below this TopN (its own order keys,
the expressions of every
+ * descendant operator and the slots that are required materialized
already) is resolved through its
+ * identity alias chain. Project expressions are handled by {@link
#collectProjectExprInputSlots},
+ * which knows that a transparent {@code Alias(Slot)} output may still be
fetched lazily.
+ *
+ * <p>A set operation is a boundary: {@link MaterializeProbeVisitor} never
reports a lazy source for a
+ * slot produced by a set operation, and {@link #collectIdentityAliasMap}
stops at it, so the aliases
+ * below a set operation are neither resolved nor reachable. If lazy
materialization is ever extended
+ * through set operations, the consumed slots have to be resolved per set
operation branch instead.
+ */
+ private void collectRequiredAliasSources(PhysicalTopN<? extends Plan> topN,
+ Set<Slot> requiredMaterializedSlots) {
+ Map<Slot, Slot> aliasToChild = new HashMap<>();
+ collectIdentityAliasMap(topN.child(), aliasToChild);
+
+ Set<Slot> consumedSlots = new HashSet<>();
+ for (OrderKey orderKey : topN.getOrderKeys()) {
+ consumedSlots.addAll(orderKey.getExpr().getInputSlots());
+ }
+ collectConsumedSlots(topN.child(), consumedSlots);
+ consumedSlots.addAll(requiredMaterializedSlots);
+ for (Slot slot : consumedSlots) {
+ collectAliasChain(slot, aliasToChild, requiredMaterializedSlots);
+ }
+ }
+
+ /** Collect the slots consumed by the expressions of the operators that
stay below the TopN. */
+ private void collectConsumedSlots(Plan plan, Set<Slot> consumedSlots) {
+ if (plan instanceof PhysicalSetOperation) {
+ // Set operations are not materialized lazily, so nothing below
them can be lazy either.
+ return;
+ }
+ if (!(plan instanceof PhysicalProject)) {
+ // Project expressions are covered by
collectProjectExprInputSlots, which keeps the input of a
+ // transparent Alias(Slot) lazy because that alias output may
still be fetched later.
+ consumedSlots.addAll(plan.getInputSlots());
Review Comment:
**[P1] Account for lateral-generate conjunct inputs** A valid reduced tree
is `TopN(sort) -> PhysicalGenerate(unnest(arr), conjunct tag = x) -> Project(a
AS x, a AS y, arr, sort) -> Scan`. `PhysicalGenerate.getExpressions()` exposes
only the generator, while the implementation rule preserves `getConjuncts()`
and the translator executes those conjuncts in `TableFunctionNode`. Therefore
this generic `getInputSlots()` walk never sees `x`; both `x` and `y` can trace
to lazy base `a`, and `LazySlotPruning` removes `a`/`x` below a conjunct that
still needs it (the validator is blind for the same reason). Please include
physical-generate conjunct inputs here or in `PhysicalGenerate.getInputSlots()`
(excluding generator outputs as the logical operator does), and add a
`fe_debug=false` lateral-UNNEST alias regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/LazyMaterializeTopN.java:
##########
@@ -292,6 +298,98 @@ private void collectProjectExprInputSlots(Plan plan,
Set<Slot> requiredMateriali
}
}
+ /**
+ * Keep the columns consumed below the TopN materialized during the scan,
resolved through identity
+ * aliases.
+ *
+ * <p>{@link MaterializeProbeVisitor} only protects the slot it is
tracing: a slot consumed by an
+ * operator on the way from the TopN down to the relation stops the probe,
but the columns an
+ * identity alias reads are never resolved. For
+ *
+ * <pre>
+ * OuterTopN(order by z)
+ * InnerTopN(order by x)
+ * Project(lazy_col AS x, lazy_col AS y, other_col AS z)
+ * OlapScan
+ * </pre>
+ *
+ * probing the outer output {@code y} resolves to the base column {@code
lazy_col}, so {@code lazy_col}
+ * is classified lazy and {@link LazySlotPruning} removes it from the
scan, while {@code lazy_col AS x}
+ * below the outer TopN is still read by the inner TopN. The plan then
references a slot its child no
+ * longer produces and the final {@link Validator} rejects it. The same
happens when an identity alias
+ * is consumed by a filter, a join condition or any other operator that
stays below the TopN.
+ *
+ * <p>Therefore every slot consumed below this TopN (its own order keys,
the expressions of every
+ * descendant operator and the slots that are required materialized
already) is resolved through its
+ * identity alias chain. Project expressions are handled by {@link
#collectProjectExprInputSlots},
+ * which knows that a transparent {@code Alias(Slot)} output may still be
fetched lazily.
+ *
+ * <p>A set operation is a boundary: {@link MaterializeProbeVisitor} never
reports a lazy source for a
+ * slot produced by a set operation, and {@link #collectIdentityAliasMap}
stops at it, so the aliases
+ * below a set operation are neither resolved nor reachable. If lazy
materialization is ever extended
+ * through set operations, the consumed slots have to be resolved per set
operation branch instead.
+ */
+ private void collectRequiredAliasSources(PhysicalTopN<? extends Plan> topN,
+ Set<Slot> requiredMaterializedSlots) {
+ Map<Slot, Slot> aliasToChild = new HashMap<>();
+ collectIdentityAliasMap(topN.child(), aliasToChild);
+
+ Set<Slot> consumedSlots = new HashSet<>();
+ for (OrderKey orderKey : topN.getOrderKeys()) {
+ consumedSlots.addAll(orderKey.getExpr().getInputSlots());
+ }
+ collectConsumedSlots(topN.child(), consumedSlots);
+ consumedSlots.addAll(requiredMaterializedSlots);
+ for (Slot slot : consumedSlots) {
+ collectAliasChain(slot, aliasToChild, requiredMaterializedSlots);
+ }
+ }
+
+ /** Collect the slots consumed by the expressions of the operators that
stay below the TopN. */
+ private void collectConsumedSlots(Plan plan, Set<Slot> consumedSlots) {
+ if (plan instanceof PhysicalSetOperation) {
+ // Set operations are not materialized lazily, so nothing below
them can be lazy either.
+ return;
+ }
+ if (!(plan instanceof PhysicalProject)) {
+ // Project expressions are covered by
collectProjectExprInputSlots, which keeps the input of a
+ // transparent Alias(Slot) lazy because that alias output may
still be fetched later.
+ consumedSlots.addAll(plan.getInputSlots());
Review Comment:
**[P1] Preserve direct index-filter predicate laziness** With
`topn_lazy_materialization_using_index=true`, `select * from t1 where user_id =
1 order by username limit 1` intentionally treats `user_id` as lazy above the
filter: `visitPhysicalFilter` returns it as a materialize source, and
`LazySlotPruning` keeps it only long enough to evaluate the predicate. This
walk adds bare `user_id`, and `collectAliasChain` then inserts that unmapped
seed into `requiredMaterializedSlots`; `collectRequiredOutputSlots`
consequently makes it eager in every TopN tuple. That contradicts the unchanged
`topNLazyMaterializationUsingIndex` shape (`lazySlots:(addr,age,user_id)`) and
will fail that regression while losing the index-mode optimization. Please
close only actual alias descendants (while preserving slots already required
for other reasons), rather than marking an unmapped direct consumer required.
--
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]
