github-actions[bot] commented on code in PR #66898:
URL: https://github.com/apache/doris/pull/66898#discussion_r4257049079
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteCteChildren.java:
##########
@@ -82,6 +82,14 @@ public Plan visit(Plan plan, CascadesContext context) {
@Override
public Plan visitLogicalCTEAnchor(LogicalCTEAnchor<? extends Plan, ?
extends Plan> cteAnchor,
CascadesContext cascadesContext) {
+ // Derive the operative slots of the producer before the statistics of
the consumer are
+ // derived below: StatsCalculator.computeOlapScan fetches the column
stats of the slots that
+ // are marked as operative, and without this derivation every column
of every scan of the
+ // producer would be fetched. The derived producer is the plan whose
statistics are derived
+ // and which is rewritten below, so that those statistics are not
computed on a discarded
+ // copy of it.
+ Plan derivedProducer = cteAnchor.child(0).accept(
+ new OperativeColumnDerive(), new
OperativeColumnDerive.DeriveContext());
Review Comment:
[P2] Include consumer keys in the producer derivation
A retained CTE can have this reduced tree (for example, `WITH c AS (SELECT a
FROM wide) SELECT a FROM c INTERSECT SELECT a FROM c`):
```text
CTEAnchor(c)
Producer -> Project(a#p) -> Scan(wide[a#p,b,...])
Intersect DISTINCT
Consumer(a#c1 -> a#p)
Consumer(a#c2 -> a#p)
```
The new context is empty, the producer has no expressions, and the
forwarding project adds no demand, so `a#p` is marked non-operative. The
prerequisite `StatsDerive` therefore only probes its cache entry; on a miss it
never requests the analyzed key statistics. Consumer derivation copies that
UNKNOWN value, and marking `a#c1`/`a#c2` in the outer pass cannot reach `a#p`
because there is no CTE demand mapping. `InferSetOperatorDistinct` then skips
local DISTINCT inference, before the producer is rewritten at line 124. A warm
cache can mask this. Please seed/map consumer estimation keys into the producer
context (or conservatively retain producer outputs for this prerequisite pass)
and cover the materialized-CTE load requests. This is distinct from the
existing late-CTE-pass thread: the pass now runs early enough but omits
downstream keys.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -589,6 +596,15 @@ public class Rewriter extends AbstractBatchJobExecutor {
bottomUp(new EliminateNotNull()),
topDown(new ConvertInnerOrCrossJoin())
),
+ // InferSetOperatorDistinct in the next topic is the first
rewrite rule that derives
+ // statistics: it calls StatsDerive on a set operation whose
statistics are missing,
+ // and StatsCalculator.computeOlapScan fetches the column
stats of the operative
+ // slots only. Derive them before that rule, so that this
consumer and every later one
+ // does not fall back to fetching the stats of all table
columns of a scan. The
+ // derivation is repeated once more before "Reorder join
before eager aggregation"
+ // and at the end of rewrite, where the operative slots of the
plans that reach the
+ // memo are computed.
+ custom(RuleType.OPERATIVE_COLUMN_DERIVE,
OperativeColumnDerive::new),
Review Comment:
[P2] Derive UNION DISTINCT deduplication keys before the stats consumer
This new pass also sees the pre-lowering tree:
```text
ResultSink(u.a)
Union DISTINCT(u.a)
Project(a#1) -> Scan(wide1[a#1,b,...])
Project(a#2) -> Scan(wide2[a#2,b,...])
```
`OperativeColumnDerive.visitLogicalUnion` only propagates parent demands and
never treats DISTINCT outputs as operative; sinks and forwarding projects add
none. Both scans are therefore marked derived-empty. On absent cache entries,
the following `InferSetOperatorDistinct` obtains UNKNOWN key NDVs through
cache-only probes and declines local deduplication, without requesting the
statistics it needs. `BuildAggForUnion` adds group-key demand only afterwards,
too late to redo that choice. This affects ordinary `SELECT a FROM wide1 UNION
SELECT a FROM wide2`; a warm cache can mask it. Please seed all DISTINCT union
keys before the early pass and add direct UNION DISTINCT versus UNION ALL
planner/load-request coverage. This is distinct from the existing
pass-placement thread: the placement is fixed, but the union visitor does not
handle this earlier DISTINCT shape.
--
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]
