github-actions[bot] commented on code in PR #68282:
URL: https://github.com/apache/doris/pull/68282#discussion_r4090300617
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -130,6 +140,94 @@ public TableStatsMeta(long rowCount, AnalysisInfo
analyzedJob, TableIf table) {
update(analyzedJob, table);
}
+ /**
+ * Create a record for a table which doesn't have one yet, in the state of
an empty table. The rows
+ * loaded into the table are accumulated by {@link
AnalysisManager#replayUpdateRowsRecord}, so a record
+ * has to exist before the first load, otherwise these rows can never be
turned into a row count.
+ */
+ public TableStatsMeta(OlapTable table) {
+ this.ctlId = table.getDatabase().getCatalog().getId();
+ this.ctlName = table.getDatabase().getCatalog().getName();
+ this.dbId = table.getDatabase().getId();
+ this.dbName = table.getDatabase().getFullName();
+ this.tblId = table.getId();
+ this.tblName = table.getName();
+ this.idxId = -1;
+ this.indexesRowCount = buildEmptyIndexRowCount(table);
+ this.updatedRowsBase.set(0);
+ }
+
+ /**
+ * TRUNCATE TABLE removes all the data of the table. Reset this record
back to the state of an empty
+ * table instead of dropping it, so that the rows loaded after the
truncation can still be accumulated
+ * into {@link #updatedRows} and be reported as the row count of the table.
+ * <p>
+ * The transition runs under the monitor of this record, which {@link
#getRowCountWithDeltaRows} also
+ * takes, so a planner which reads the row count of the table without
holding its lock either sees the
+ * whole transition or none of it. The order the fields are published in
matters for the readers which
+ * don't take the monitor, for instance SHOW TABLE STATS: the baseline
first makes the delta empty while
+ * the collected row count is still the one of the removed data, so the
emptied row count is only
+ * published once no row of the removed data is counted as a delta row
anymore.
+ */
+ public synchronized void reset(OlapTable table) {
+ updatedRowsBase.set(updatedRows.get());
+ indexesRowCount = buildEmptyIndexRowCount(table);
+ updatedRows.set(0);
+ // None of the rows loaded from now on is included in the collected
row count. They are all delta rows.
+ updatedRowsBase.set(0);
+ rowCount = 0;
+ partitionUpdateRows.clear();
+ // Drop the column statistics baseline: the row count captured by the
previous analysis described
+ // the removed data, it must not cancel out the rows loaded after the
truncation.
+ colToColStatsMeta.clear();
+ // The statistics of the removed data is stale, let the analyzer
collect it again.
+ partitionChanged.set(true);
+ // The injected statistics described the removed data, it no longer
applies to this table.
+ userInjected = false;
+ // The emptied table has never been analyzed, and no analyze job
describes it any more.
+ updatedTime = 0;
+ lastAnalyzeTime = 0;
+ jobType = null;
+ }
+
+ private static ConcurrentMap<Long, Long> buildEmptyIndexRowCount(OlapTable
table) {
+ // TRUNCATE TABLE removed the data of every index of the table, so
every index whose row count is known
+ // to follow the row count of the base index is known to be empty. The
row count of an index which
+ // aggregates is unknown until the backends report it, so it is not
claimed to be 0 here.
+ ConcurrentMap<Long, Long> indexRowCount = new ConcurrentHashMap<>();
+ for (Long indexId : table.getIndexIdList()) {
+ if (keepsOneRowPerBaseRow(table, indexId)) {
+ indexRowCount.put(indexId, 0L);
+ }
+ }
+ return indexRowCount;
+ }
+
+ /**
+ * Whether the rows loaded into the base index are the rows of this index
as well. That holds for the base
+ * index itself and for an index which keeps one row per base row, i.e. a
duplicate key index whose columns
+ * are all plain. An index which aggregates, or which merges the rows of a
unique key table, has a smaller
+ * row count of its own, so the rows loaded into the base index must not
be added to it.
+ */
+ public static boolean keepsOneRowPerBaseRow(OlapTable table, long indexId)
{
+ if (indexId == table.getBaseIndexId()) {
+ return true;
+ }
+ MaterializedIndexMeta indexMeta = table.getIndexMetaByIndexId(indexId);
+ if (indexMeta == null || indexMeta.getKeysType() != KeysType.DUP_KEYS)
{
Review Comment:
[P2] Exclude filtered DUP_KEYS materialized indexes
A filter-only synchronous MV inherits `DUP_KEYS` and can have only
null/`NONE` aggregation types, but it does not keep one row per base row:
`MaterializedIndexMeta.getWhereClause()` is persisted and sent through both
sink paths. After a truncate, loading 100 base rows of which one satisfies the
MV predicate makes this helper seed the MV at zero and both planner consumers
report a cardinality/delta of 100 while the index contains one row and its
strict BE count is unavailable. The prior thread covers an unfiltered
projection rollup, so this is a distinct supported shape. Please require a null
`whereClause` (or track a predicate-aware index delta) and add a filtered
projection-MV regression.
##########
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:
[P2] Pair each rollup count with its own baseline
The delta here is anchored at the base index's `updatedRowsBase`, but a
row-preserving rollup count need not describe that snapshot. For example, after
a base analysis at 100 and 50 more loaded rows, a rollup-only analysis stores
150 while `update()` deliberately leaves the base baseline at 100; during a
strict-count gap this returns `150 + (150 - 100) = 200`. A rollup published
after reset has the other failure: it was never seeded, so after three rows
load the fallback is `-1 + 3 = 2`. This is distinct from the existing thread's
rollup that was already present and seeded at reset. Please keep count/baseline
provenance per index, never add a delta to `UNKNOWN_ROW_COUNT`, and cover later
rollup publication plus rollup-only analysis.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -130,6 +140,94 @@ public TableStatsMeta(long rowCount, AnalysisInfo
analyzedJob, TableIf table) {
update(analyzedJob, table);
}
+ /**
+ * Create a record for a table which doesn't have one yet, in the state of
an empty table. The rows
+ * loaded into the table are accumulated by {@link
AnalysisManager#replayUpdateRowsRecord}, so a record
+ * has to exist before the first load, otherwise these rows can never be
turned into a row count.
+ */
+ public TableStatsMeta(OlapTable table) {
+ this.ctlId = table.getDatabase().getCatalog().getId();
+ this.ctlName = table.getDatabase().getCatalog().getName();
+ this.dbId = table.getDatabase().getId();
+ this.dbName = table.getDatabase().getFullName();
+ this.tblId = table.getId();
+ this.tblName = table.getName();
+ this.idxId = -1;
+ this.indexesRowCount = buildEmptyIndexRowCount(table);
+ this.updatedRowsBase.set(0);
+ }
+
+ /**
+ * TRUNCATE TABLE removes all the data of the table. Reset this record
back to the state of an empty
+ * table instead of dropping it, so that the rows loaded after the
truncation can still be accumulated
+ * into {@link #updatedRows} and be reported as the row count of the table.
+ * <p>
+ * The transition runs under the monitor of this record, which {@link
#getRowCountWithDeltaRows} also
+ * takes, so a planner which reads the row count of the table without
holding its lock either sees the
+ * whole transition or none of it. The order the fields are published in
matters for the readers which
+ * don't take the monitor, for instance SHOW TABLE STATS: the baseline
first makes the delta empty while
+ * the collected row count is still the one of the removed data, so the
emptied row count is only
+ * published once no row of the removed data is counted as a delta row
anymore.
+ */
+ public synchronized void reset(OlapTable table) {
+ updatedRowsBase.set(updatedRows.get());
+ indexesRowCount = buildEmptyIndexRowCount(table);
+ updatedRows.set(0);
+ // None of the rows loaded from now on is included in the collected
row count. They are all delta rows.
+ updatedRowsBase.set(0);
+ rowCount = 0;
+ partitionUpdateRows.clear();
+ // Drop the column statistics baseline: the row count captured by the
previous analysis described
+ // the removed data, it must not cancel out the rows loaded after the
truncation.
+ colToColStatsMeta.clear();
+ // The statistics of the removed data is stale, let the analyzer
collect it again.
+ partitionChanged.set(true);
+ // The injected statistics described the removed data, it no longer
applies to this table.
+ userInjected = false;
+ // The emptied table has never been analyzed, and no analyze job
describes it any more.
+ updatedTime = 0;
+ lastAnalyzeTime = 0;
+ jobType = null;
+ }
+
+ private static ConcurrentMap<Long, Long> buildEmptyIndexRowCount(OlapTable
table) {
+ // TRUNCATE TABLE removed the data of every index of the table, so
every index whose row count is known
+ // to follow the row count of the base index is known to be empty. The
row count of an index which
+ // aggregates is unknown until the backends report it, so it is not
claimed to be 0 here.
+ ConcurrentMap<Long, Long> indexRowCount = new ConcurrentHashMap<>();
+ for (Long indexId : table.getIndexIdList()) {
+ if (keepsOneRowPerBaseRow(table, indexId)) {
+ indexRowCount.put(indexId, 0L);
+ }
+ }
+ return indexRowCount;
+ }
+
+ /**
+ * Whether the rows loaded into the base index are the rows of this index
as well. That holds for the base
+ * index itself and for an index which keeps one row per base row, i.e. a
duplicate key index whose columns
+ * are all plain. An index which aggregates, or which merges the rows of a
unique key table, has a smaller
+ * row count of its own, so the rows loaded into the base index must not
be added to it.
+ */
+ public static boolean keepsOneRowPerBaseRow(OlapTable table, long indexId)
{
+ if (indexId == table.getBaseIndexId()) {
Review Comment:
[P2] Keep merge-key base indexes out of this fallback
This base-index shortcut runs before the key-type check, so a `UNIQUE_KEYS`
or `AGG_KEYS` base scan receives the physical rowset delta as its logical
cardinality. After `TRUNCATE`, two rapid transactions writing the same full key
contribute two `rowset_meta.num_rows()` entries to `updatedRows`; while the new
tablets' strict count is still unknown, this getter and
`computeDeltaRowCount()` report 2 even though the scan merges that key to one
row. The current aggregate regression uses distinct full keys, and the existing
threads cover non-base rollups, so neither exercises this base-index case.
Please classify the base index by key semantics too (or leave the fallback
unknown/carry a logical delta) and cover repeated keys across transactions for
UNIQUE and AGG bases.
--
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]
