englefly commented on code in PR #68282:
URL: https://github.com/apache/doris/pull/68282#discussion_r4090120147


##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +374,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 (indexId != table.getBaseIndexId()) {

Review Comment:
   Fixed at `4001d5d4532`. An index which is proven row-preserving now gets the 
rows loaded after the truncation, in both consumers you named.
   
   - `TableStatsMeta.keepsOneRowPerBaseRow(table, indexId)`: the base index 
itself, or a `DUP_KEYS` index none of whose columns aggregates. One detail 
worth recording, because I got it wrong first: the *key* columns of an index 
carry no aggregation type at all (`Column.java:499`), the value columns of a 
duplicate key index are `NONE`. My first version compared against `NONE` only 
and therefore rejected `r_dup` on its key column; the live run (cardinality 
stayed 1) caught it, and the predicate now treats `null` and `NONE` as "no 
aggregation".
   - `buildEmptyIndexRowCount()` seeds such an index with an empty row count 
when a truncation resets the record (`TRUNCATE` empties every index of the 
table), and `getRowCountWithDeltaRows()` adds the rows loaded since the 
truncation to it, so `OlapScan(t, selectedIndex=r_dup)` no longer gets `-1` 
from the guard. `StatsCalculator.computeDeltaRowCount()` uses the same 
predicate, so the loaded rows are visible to filter estimation as well.
   - An index which aggregates, or which merges the rows of a unique key table, 
is untouched: it keeps its own row count and never gets the base delta (the 
aggregate-rollup case of the suite still passes).
   - Tests: 
`TableStatsMetaTest#testRollupKeepingOneRowPerBaseRowReportsTheRowsLoadedAfterTheTruncate`
 and `#testAggregatingRollupDoesNotReportTheRowsLoadedAfterTheTruncate` (the 
mocked index schemas mirror the real column shapes), and a post-truncate 
projection-rollup case in 
`regression-test/suites/statistics/test_row_count_after_truncate.groovy` which 
scans the rollup directly with `SELECT k1, v FROM ... INDEX r_dup`.
   - Verified on a local FE and BE cluster, reproducing your case: `CREATE 
TABLE t2 (k1, k2, v) DUPLICATE KEY(k1, k2)`, `ALTER TABLE t2 ADD ROLLUP r_dup 
(k1, v)`, `TRUNCATE TABLE t2`, then 3 rows inserted. `EXPLAIN SELECT k1, v FROM 
t2 INDEX r_dup` reported `cardinality=1` before this commit and reports 
`cardinality=3` after it, with `StatsCalculator.computeOlapScan` logging 
`tableRowCount is 1.0` before and `3.0` after, while `EXPLAIN SELECT * FROM t2` 
stays at 3.
   



-- 
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]

Reply via email to