englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4312382722
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -194,6 +273,21 @@ public void update(AnalysisInfo analyzedJob, TableIf
tableIf) {
if (tableIf != null) {
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
+ // The collected row counts published below already include
the rows which had been loaded when
+ // the job was built, remember how many they were, they are
not delta rows. The baseline may
+ // only advance together with the collected base index row
count, an analysis of another index
+ // (a materialized view) doesn't touch it.
+ // NOTICE: the baseline is published before the collected
counts. They are both read without
+ // the table lock by the planner, and a reader which pairs the
collected count of this analysis
+ // with the baseline of the previous one counts the rows
loaded since that previous analysis
+ // twice.
+ if
(analyzedJob.indexesRowCount.containsKey(olapTable.getBaseIndexId())) {
+ // A row count supplied by the user replaces the collected
one and describes the table as
+ // of now, so the rows loaded so far are part of it and
the baseline has to move to the
+ // current value. Keeping the older baseline would count
those rows again once DROP STATS
+ // clears userInjected.
+ updatedRowsBase.set(analyzedJob.userInject ?
updatedRows.get() : analyzedJob.updateRows);
Review Comment:
Agreed that the two values describe different snapshots, but this one is
pre-existing rather than a regression from this change: before the
record-creation work the baseline was derived from `ColStatsMeta.updatedRows`,
which is also `analyzedJob.updateRows`, captured in
`AnalysisManager#buildAnalysisInfoForTbl` (line 428) when the job is built,
while the count comes from the task's `COUNT(1)` executed later.
`updatedRowsBase` keeps exactly the same capture point, so master computes the
same wrong estimate (`150 + (150 - 100) = 200`).
Tracked separately in #69281. The fix is to capture the load counter when
the analyze query collects the row count (after the query, so the residual race
biases to an under-count instead of a double count) and use it as the baseline,
plus a deterministic "build job -> load -> count" test. The test added here
only covers a stale snapshot delivered after the record was recreated, where
the baseline and the collected count cancel out.
--
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]
