github-actions[bot] commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r3999825545
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -577,7 +587,26 @@ public Optional<ConnectorMvccSnapshot> beginQuerySnapshot(
Identifier identifier =
Identifier.create(paimonHandle.getDatabaseName(), paimonHandle.getTableName());
long id = latestSnapshotCache.getOrLoad(identifier,
() ->
catalogOps.latestSnapshotId(resolveTable(paimonHandle)).orElse(-1L));
- return
Optional.of(ConnectorMvccSnapshot.builder().snapshotId(id).build());
+ return Optional.of(ConnectorMvccSnapshot.builder().snapshotId(id)
+ .schemaId(statementSchemaId(paimonHandle,
resolveTable(paimonHandle))).build());
Review Comment:
[P1] Avoid resolving ordinary latest schemas through this generation-blind
schema-ID memo. This new schema ID makes `materializeLatest` call
`getTableSchema(..., snapshot)`, whose `PaimonSchemaAtMemo` has no TTL and is
keyed only by db/table/sys/branch/schemaId. After an external drop/recreate,
the normal table and latest caches can reload generation B while the memo still
holds generation A's schema 0; if B also starts at schema 0, metadata returns A
but `resolveScanTable` reads B, and restoration is skipped because the numeric
IDs match. This can leave the recreated table bound to the old schema
indefinitely until an explicit Doris invalidation. Please retain the
statement's actual latest schema value or include a stable table-generation
identity in the memo/pin, and cover schema-ID reuse after external recreation.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -483,33 +484,39 @@ public ConnectorTableSchema getTableSchema(
return getTableSchema(session, handle);
}
Table table = loadTable(session, iceHandle);
- Schema schema;
- if (table.currentSnapshot() == null) {
- // Empty table: legacy getSchema falls back to the latest schema
(NEWEST_SCHEMA_ID path).
- schema = table.schema();
- } else {
- schema = table.schemas().get((int) snapshot.getSchemaId());
- if (schema == null) {
- // Defensive: a pinned id absent from table.schemas() (legacy
would NPE) -> latest.
- // INVARIANT: this SLOT-schema fallback MUST stay identical to
the DICT-schema fallback in
- // IcebergScanPlanProvider.pinnedSchema (same getSchemaId()
lookup + same silent -> table.schema()).
- // If the two diverge, the field-id dict names and the BE
scan-slot names resolve DIFFERENT
- // schemas -> BE children.at() std::out_of_range-SIGABRT on a
schema-evolved time-travel read
- // (reverify #65185 L16). Do not harden ONE side to throw
without the other.
- schema = table.schema();
- }
+ Schema schema = resolvePinnedSchema(table, snapshot);
+ String specId =
snapshot.getProperties().get(PARTITION_SPEC_ID_PROPERTY);
+ PartitionSpec spec = specId == null ? table.spec() :
table.specs().get(Integer.parseInt(specId));
Review Comment:
[P1] Also guard against a cached spec ID being reused by a replacement
table. The cache is keyed only by `TableIdentifier` and carries no table UUID,
so after an external drop/recreate a fresh generation B can have its own spec
1. In that case `table.specs().get(1)` is non-null and this new fallback
accepts B's unrelated historical spec instead of B's current spec. For example,
A can cache schema 5/spec 1, while B lacks schema 5 (so the adjacent lookup
falls back to B current) but has advanced through spec 1 to spec 2; schema
assembly then pairs B's current schema with B's wrong old spec. This is
distinct from the covered missing-ID case because the ID exists. Please
validate a stable table-generation identity before consuming cached schema/spec
IDs and add a recreate test that reuses the spec ID before advancing again.
--
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]
