github-actions[bot] commented on code in PR #68135:
URL: https://github.com/apache/doris/pull/68135#discussion_r4035935503
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -422,13 +420,30 @@ public boolean branchExists(Table table, String
branchName) {
@Override
public long rowCount(Table table) {
- // Legacy PaimonExternalTable.fetchRowCount /
PaimonSysExternalTable.fetchRowCount: sum
- // the planned-split record counts.
- long rowCount = 0;
- for (Split split :
table.newReadBuilder().newScan().plan().splits()) {
- rowCount += split.rowCount();
+ // System/format tables have no data snapshot count. A fallback
pair combines two
+ // branches, so its main snapshot alone cannot estimate the
relation either.
+ if (!(table instanceof FileStoreTable)
+ ||
PaimonTableDecorators.unwrapToFallbackOrBase((FileStoreTable) table)
+ instanceof FallbackReadFileStoreTable) {
+ return -1;
}
- return rowCount;
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
+ switch (fileStoreTable.coreOptions().startupMode()) {
Review Comment:
[P2] Account for intrinsic batch-scan filters
An allowed startup mode does not guarantee that the batch scan reads every
file counted by `Snapshot.totalRecordCount`. In Paimon 1.3.1, primary-key
tables with deletion vectors or `FIRST_ROW` apply a `level > 0` filter when
`batch-scan-mode` is `NONE`, and `POSTPONE_BUCKET` tables always apply a
real-bucket-only filter, even under these full-snapshot modes. The stored total
is maintained from all manifest add/delete row counts and applies neither
filter, whereas the removed `newScan().plan()` sum reflected the visible file
set. Please return UNKNOWN when either effective filter applies (or derive the
estimate from the same filtered projection) and add real committed-file
coverage; the synthetic primary-key snapshot cannot expose this divergence.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -422,13 +420,30 @@ public boolean branchExists(Table table, String
branchName) {
@Override
public long rowCount(Table table) {
- // Legacy PaimonExternalTable.fetchRowCount /
PaimonSysExternalTable.fetchRowCount: sum
- // the planned-split record counts.
- long rowCount = 0;
- for (Split split :
table.newReadBuilder().newScan().plan().splits()) {
- rowCount += split.rowCount();
+ // System/format tables have no data snapshot count. A fallback
pair combines two
+ // branches, so its main snapshot alone cannot estimate the
relation either.
+ if (!(table instanceof FileStoreTable)
+ ||
PaimonTableDecorators.unwrapToFallbackOrBase((FileStoreTable) table)
+ instanceof FallbackReadFileStoreTable) {
+ return -1;
}
- return rowCount;
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
+ switch (fileStoreTable.coreOptions().startupMode()) {
+ case LATEST:
+ case LATEST_FULL:
+ case FROM_TIMESTAMP:
+ case FROM_SNAPSHOT:
+ case FROM_SNAPSHOT_FULL:
+ break;
+ default:
+ // Incremental/file-creation-time scans and unresolved
compacted-full scans
+ // do not read the full snapshot selected by
TimeTravelUtil.
+ return -1;
+ }
+ Snapshot snapshot =
TimeTravelUtil.tryTravelOrLatest(fileStoreTable);
Review Comment:
[P2] Preserve SELECT-only statistics access
`TimeTravelUtil.tryTravelOrLatest(fileStoreTable)` eagerly obtains
`table.tagManager()` even for an ordinary latest read. On Paimon 1.3.1's
`PrivilegedFileStoreTable`, `tagManager()` requires INSERT, whereas the removed
`newScan().plan()` path requires only SELECT. Whenever runtime normalization
preserves that wrapper (for example, an already-safe manifest setting), a
SELECT-only catalog user now throws here; `getTableStatistics` catches it and
returns UNKNOWN and logs a warning, so a valid read loses the row-count
estimate this override is meant to provide. Please preserve the wrapper's
SELECT authorization semantics while reading snapshot metadata, and cover the
preserved privileged-table shape with a SELECT-only checker.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -422,13 +420,30 @@ public boolean branchExists(Table table, String
branchName) {
@Override
public long rowCount(Table table) {
- // Legacy PaimonExternalTable.fetchRowCount /
PaimonSysExternalTable.fetchRowCount: sum
- // the planned-split record counts.
- long rowCount = 0;
- for (Split split :
table.newReadBuilder().newScan().plan().splits()) {
- rowCount += split.rowCount();
+ // System/format tables have no data snapshot count. A fallback
pair combines two
+ // branches, so its main snapshot alone cannot estimate the
relation either.
+ if (!(table instanceof FileStoreTable)
+ ||
PaimonTableDecorators.unwrapToFallbackOrBase((FileStoreTable) table)
+ instanceof FallbackReadFileStoreTable) {
+ return -1;
}
- return rowCount;
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
+ switch (fileStoreTable.coreOptions().startupMode()) {
+ case LATEST:
+ case LATEST_FULL:
+ case FROM_TIMESTAMP:
+ case FROM_SNAPSHOT:
+ case FROM_SNAPSHOT_FULL:
+ break;
+ default:
+ // Incremental/file-creation-time scans and unresolved
compacted-full scans
Review Comment:
[P2] Preserve the file-creation filter in statistics
Doris-normalized `@options` file-creation reads do reach this branch.
`resolveFileCreationTime` replaces the public partial-read key with
`scan.snapshot-id` and carries the threshold only in the private
`doris.internal.paimon.file-creation-time-millis` handle marker. The statistics
path then calls `applyOptions`, which strips that marker, so this switch sees
`FROM_SNAPSHOT` and returns the whole snapshot count; execution still uses the
marker in `planFileCreationTimeSplits` to filter manifests by creation time.
This overestimates the relation compared with both the scan and the removed
split sum. Please return UNKNOWN for this resolved marker (including the
creation-time fallback) and cover the normalized handle path rather than only a
raw Paimon option copy.
--
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]
