englefly commented on code in PR #66898:
URL: https://github.com/apache/doris/pull/66898#discussion_r4089175753
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -620,6 +637,22 @@ public Statistics computeOlapScan(OlapScan olapScan) {
return computeVirtualColumnStats(olapScan, builder.build());
}
+ /**
+ * Returns the slots whose column stats should be fetched for the query.
+ *
+ * <p>Only operative slots' column stats are needed by the query, column
stats of other slots are
+ * useless and fetching them would pollute the column stats cache and
waste time on wide tables.
+ * If operative slots are not derived yet (e.g. stats derivation during
RBO) or full stats
+ * fidelity is required (forbidUnknownColStats), fall back to all output
slots.
+ */
+ private List<Slot> getStatsNeededSlots(OlapScan olapScan) {
+ if (forbidUnknownColStats) {
Review Comment:
Fixed. The state of the derivation is now kept apart from the slot list:
`CatalogRelation#isOperativeSlotsDerived()` records it, set by the
`withOperativeSlots` of the relations that derive them (`LogicalOlapScan`,
`LogicalOlapTableStreamScan`, `LogicalFileScan`), and `LogicalPlanDeepCopier`
no longer marks a copy as derived when the plan it copied from was not derived.
`getStatsNeededSlots` now fetches every output slot only when full stats
fidelity is required (`forbid_unknown_col_stats`, which has to load the
statistics to tell "no statistics" from "not loaded") and when the operative
slots have not been derived yet. A derived empty list is neither of those: it
means the query needs no column of the relation, so nothing is fetched.
Coverage:
*
`StatsCalculatorTest#testComputeOlapScanWithDerivedEmptyOperativeSlotsFetchesNoColumnStats`:
a scan with two output columns and a derived empty operative list triggers no
column stats load at all, while the same scan without the derivation still
fetches both columns (the pre-existing fallback).
*
`OperativeColumnDeriveTest#testPhysicalResultSinkDoesNotMarkOutputAsOperativeSlots`
now also asserts that the derived empty list is marked as derived.
* `shape_check/tpcds_sf100` (395 suites) passes with `fe_debug = true`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -609,7 +619,14 @@ public Statistics computeOlapScan(OlapScan olapScan) {
} else {
// get table level stats
for (SlotReference slot : visibleOutputSlots) {
- ColumnStatistic cache =
olapTableStats.getColumnStatistics(slot.getName(), connectContext);
+ ColumnStatistic cache;
+ if (!statsNeededSlots.contains(slot)) {
Review Comment:
Addressed in the follow-up PR that this branch stacks on, which is about the
width of columns whose statistics are not fetched. There, a slot the query does
not need is unknown and as wide as its data type
(`ColumnStatistic#createUnknownByDataType`) instead of being read from the
cache, so its width no longer depends on which columns another query happened
to load.
`StatisticsCache#getColumnStatisticsIfPresent` is removed by that change,
which also covers the cache probe issue below.
It is kept out of this PR on purpose: it is an estimation/width change, not
a change of which statistics are fetched. It moves the join order of
`shape/query4`, `shape/query14`, `rf_prune/query4`, `rf_prune/query14` and
`noStatsRfPrune/query24`, whereas this PR alone changes no plan shape: 395/395
suites of `shape_check/tpcds_sf100` pass, deterministically with and without
the fuzzy session variables.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/cache/StatisticsCache.java:
##########
@@ -136,6 +155,28 @@ private ColumnStatistic doGetColumnStatistics(
return ColumnStatistic.UNKNOWN;
}
+ /**
+ * Returns the column statistic only when it is already present in the
cache, without
+ * triggering a cache load. Used for columns whose stats are not needed by
the query,
+ * so that useless columns do not pollute the cache nor issue queries to
the stats table.
+ */
+ private ColumnStatistic doGetColumnStatisticsIfPresent(
+ long catalogId, long dbId, long tblId, long idxId, String colName,
ConnectContext ctx) {
+ StatisticsCacheKey k = new StatisticsCacheKey(catalogId, dbId, tblId,
idxId, colName);
+ CompletableFuture<Optional<ColumnStatistic>> f =
columnStatisticsCache.getIfPresent(k);
Review Comment:
Covered by the follow-up width change:
`StatisticsCache#getColumnStatisticsIfPresent` and
`doGetColumnStatisticsIfPresent` are removed there, so the `refreshAfterWrite`
-> `afterRead` -> loader path and the access policy side effect of the probe
cannot happen any more. No quiet replacement probe is needed, because the
caller no longer reads the cache for such a slot at all: it uses unknown
statistics that carry the width of the data type.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -678,6 +685,13 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(new PushDownAggThroughJoinOnPkFk()),
topDown(new PullUpJoinFromUnionAll())
),
+ // RBO rules that depend on statistics (e.g. InitJoinOrder,
SkewJoin, Eager
+ // aggregation, DecomposeRepeatWithPreAggregation,
DistinctAggStrategySelector)
+ // must be placed AFTER OperativeColumnDerive:
StatsCalculator.computeOlapScan
+ // only fetches column stats of operative slots, so rules
running before the
+ // derivation would fetch stats of all table columns,
polluting the column stats
+ // cache and wasting time on wide tables.
+ custom(RuleType.OPERATIVE_COLUMN_DERIVE,
OperativeColumnDerive::new),
Review Comment:
Added. The rewriter now derives the operative slots before the "Set
operation optimization" topic, i.e. before `InferSetOperatorDistinct`, which is
the first rewrite rule that derives statistics (`StatsDerive` on a set
operation whose statistics are missing).
The derivation is repeated before "Reorder join before eager aggregation"
and at the end of rewrite, so the passes are now: materialized view pre
rewrite, this one, before the eager aggregation topic, and the end of rewrite.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -415,6 +415,13 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(new LimitSortToTopN()),
topDown(new SplitLimit()),
custom(RuleType.SET_PREAGG_STATUS,
SetPreAggStatus::new),
+ // Derive operative columns on the plan
recorded for materialized view
+ // pre rewrite: the pre rewrite runs a
cost-based optimization on this
+ // recorded plan to choose the best
materialized view, and without
+ // operative slots computeOlapScan would
fall back to fetching column
+ // stats of all table columns. The
derivation is repeated before "init
+ // join" and at the end of rewrite, where
the operative slots of the
+ // plans finally stored into the memo are
recomputed.
custom(RuleType.OPERATIVE_COLUMN_DERIVE,
OperativeColumnDerive::new),
Review Comment:
Fixed in `RewriteCteChildren#visitLogicalCTEAnchor`: the operative slots of
the producer are derived before the prerequisite `StatsDerive`, and on the plan
whose statistics are computed and which is rewritten afterwards, so those
statistics are not attached to a discarded copy of the producer.
This covers the ordinary CTE path and the materialized view pre rewrite,
which share this method.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -826,6 +841,11 @@ public class Rewriter extends AbstractBatchJobExecutor {
)
),
topDown(new CollectCteConsumerOutput()),
+ // Re-derive operative columns at the end of rewrite: rules
after the early
+ // OperativeColumnDerive (before "init join") may rebuild
scans or add virtual
+ // columns (e.g. stream scan normalization, variant virtual
column push down),
Review Comment:
Added: the rewriter derives the operative slots once more right after the
"Table/Physical optimization" topic, which contains
`NormalizeOlapTableStreamScan`, and therefore before the "set initial join
order" topic that runs `SkewJoin`. The pass at the end of rewrite is kept for
the rules that add columns afterwards (variant virtual column push down).
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -690,6 +690,13 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(new PushDownAggThroughJoinOnPkFk()),
topDown(new PullUpJoinFromUnionAll())
),
+ // RBO rules that depend on statistics (e.g. InitJoinOrder,
SkewJoin, Eager
+ // aggregation, DecomposeRepeatWithPreAggregation,
DistinctAggStrategySelector)
+ // must be placed AFTER OperativeColumnDerive:
StatsCalculator.computeOlapScan
+ // only fetches column stats of operative slots, so rules
running before the
+ // derivation would fetch stats of all table columns,
polluting the column stats
+ // cache and wasting time on wide tables.
+ custom(RuleType.OPERATIVE_COLUMN_DERIVE,
OperativeColumnDerive::new),
Review Comment:
补一下这轮改动后的最终状态。现在共 5 遍
`OperativeColumnDerive`，确实都不能省；而且为了保证中间的统计消费者不会退回"取全列统计"，还新增/前移了两遍：
1. MV pre-rewrite（MV job list 内）：MV 选型会对记录下来的 plan 跑一次 CBO，没有 operative
slots 就会退回全列取统计；
2. **新增**在 "Set operation optimization" 之前：`InferSetOperatorDistinct` 是
rewrite 期第一个统计消费者（缺统计时会调 `StatsDerive`）；
3. 原来那遍：rebase 到最新 master 后，上游已经把旧的 "init join" topic 换成了 "Reorder join
before eager aggregation"（`ReorderJoinBeforeEagerAgg`），这遍就挂在新 topic 之前，给依赖统计的
RBO 规则用；
4. **新增**在 "Table/Physical optimization" 之后：该 topic 里的
`NormalizeOlapTableStreamScan` 会重建扫描节点（新节点 operative 为空），而 `SkewJoin` 在后面的 "set
initial join order" topic 里会对没有统计的孩子推导统计；
5. rewrite 末尾：variant 虚拟列下推等规则会新增列，需要最终重算，供 CBO 统计推导和 BE lazy
materialization 使用。
另外，为了让 `select count(*) from wide_table`
这种"推导结果本来就是空"的情况一列统计都不取，`getStatsNeededSlots` 现在用 `isOperativeSlotsDerived()`
区分"已推导且为空"和"还没推导"，不再靠列表是否为空来判断。
--
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]
