englefly commented on code in PR #68282:
URL: https://github.com/apache/doris/pull/68282#discussion_r4118253687
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +406,24 @@ public long getBaseIndexDeltaRowCount(OlapTable table) {
return updatedRows.get() - maxUpdateRows;
}
+ /**
+ * The row count of the index together with the rows loaded since it was
collected, i.e. the row count of
+ * the table. Both are read by the planner without the table lock, for
instance while it plans a direct
+ * scan of a materialized view, so they have to come from the same state
of this record: a collected row
+ * count paired with the baseline of another analysis, or of a truncation,
would count rows twice or miss
+ * them. The rows loaded while this call runs are not part of the
snapshot, whichever state reads them
+ * accumulates them in {@link #updatedRows} and reports them as delta rows.
+ */
+ public synchronized long getRowCountWithDeltaRows(OlapTable table, long
indexId) {
+ long rowCount = getRowCount(indexId);
+ if (!keepsOneRowPerBaseRow(table, indexId)) {
+ // The index aggregates, or it merges the rows of a unique key
table: it has its own, smaller row
+ // count, and the rows loaded into the base index would overstate
it.
+ return rowCount;
+ }
+ return rowCount + getBaseIndexDeltaRowCount(table);
Review Comment:
Both halves agree with what I measured, one of them reproduced exactly.
**(b) rollup published after the reset - reproduced, and your arithmetic is
exact:**
```
CREATE TABLE t3 (k1 INT, k2 INT, v INT) DUPLICATE KEY(k1, k2) ...;
INSERT INTO t3 VALUES (1,1,1),(2,2,2),(3,3,3);
TRUNCATE TABLE t3; -- reset seeds only the
indexes which exist then
ALTER TABLE t3 ADD ROLLUP r_late (k1, v); -- created after the reset
-> never seeded
INSERT INTO t3 VALUES (1,1,1),(2,2,2),(3,3,3);
EXPLAIN SELECT k1, v FROM t3 INDEX r_late; -- cardinality=2 <-- -1 + 3
EXPLAIN SELECT * FROM t3; -- cardinality=3
```
`getRowCount(r_late)` is `-1` (never seeded) and the guard then adds the
delta to it. "Never add a delta to `UNKNOWN_ROW_COUNT`" is right, and that
guard belongs in `getRowCountWithDeltaRows()` regardless of how the index got
there.
**(a) rollup analysed on its own - not reproduced live, but the code path is
exactly as you describe:** the base analysis publishes `indexesRowCount[base] =
100` and moves `updatedRowsBase` to 100, 50 more rows loaded make `updatedRows
= 150`, and an analysis which carries only the rollup index stores
`indexesRowCount[rollup] = 150` while `update()` deliberately leaves the base
baseline at 100 (it only advances it for a job which contains the base index
row count, from the earlier review about injected counts). A rollup scan then
reads `150 + (150 - 100) = 200`.
Fix I intend, once the design is settled: keep count and baseline **per
index** - a published index row count moves that index's own baseline with it,
and the delta is only applied to the index whose baseline it belongs to - plus
the unknown guard above. I am not changing code in this round; the plan is to
land all three findings (this one, the merge-key base index and the filtered
index) in one commit with the four missing test shapes (repeated full keys on
`UNIQUE`/`AGG`, a rollup added after the reset, a rollup-only analysis, a
filtered projection MV) so the ordering and provenance rules are stated once.
--
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]
