englefly commented on code in PR #68019:
URL: https://github.com/apache/doris/pull/68019#discussion_r4042710275
##########
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:
Refined in ede4daaef76, exactly along the lines you suggested.
`MaterializeProbeVisitor.isIndexLazyFilter(filter)` is now the single
definition of the shape (index mode on + filter directly above an OLAP scan +
table supports TopN lazy materialization), and all three places call it:
- `MaterializeProbeVisitor.visitPhysicalFilter` - reports those slots as
lazy sources (the L91 branch);
- `LazySlotPruning.visitPhysicalFilter` - keeps them out of the scan's lazy
slots;
- `LazyMaterializeTopN` - must not require them materialized, i.e. the
consumed-slot walk skips them and resolves everything else through the alias
chain again.
This is stronger than the previous formulation for two reasons:
1. Skipping those slots provably cannot starve anything below the TopN:
`LazySlotPruning.visitPhysicalFilter` removes exactly those slots from the
scan's lazy slots, so the scan keeps producing them for the predicate. With the
old "only resolve slots that take part in an alias chain" rule I had to argue
per operator that the probe stops for direct consumers; now the exemption is a
property of one shared predicate.
2. Every other consumed slot (inner TopN order key, join condition, generate
conjunct, ...) is protected unconditionally again, so the bare-column conjunct
shape (`on tt.tag = s.lazy_col`) does not depend on a separate alias-source
argument.
While extracting the helper I also fixed the stale comment in
`LazySlotPruning.visitPhysicalFilter`: it claimed the after-state is
`materializeOlapScan(rowid, lazy=[a, c], ...)`, but the code drops the
predicate slot from the scan's lazy slots, i.e. `lazy=[c]` - the scan keeps
producing `a` for the predicate, which is precisely why the exemption is safe.
Plan output is unchanged: `query_p0/topn_lazy` (8 suites, expected output
untouched), `TopnLazyMaterializeTest` + `LazyMaterializeTopNTest` (12 tests),
and the shape checks (`materializedSlots:(t1.username)
lazySlots:(t1.addr,t1.age,t1.user_id)`, `materializedSlots:(x, lazy_col)
lazySlots:(other_col)`, both lateral unnest shapes) all pass.
--
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]
