Gabriel39 commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r4011132386
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -172,6 +172,19 @@ private PluginDrivenMvccSnapshot materializeLatest(
// legacy listPartitions/LIST/timestamp path below (byte-unchanged;
the no-op applySnapshot for the
// latest pin is side-effect-free for both paimon and iceberg).
ConnectorTableHandle pinnedHandle = metadata.applySnapshot(session,
handle, connectorSnapshot);
+ PluginDrivenSchemaCacheValue pinnedSchema = null;
+ if (connectorSnapshot.getSchemaId() >= 0) {
+ // Latest data and schema can advance independently. Keep the
connector's exact schema
+ // on the statement pin so analysis cannot fall back to a
different cached generation.
+ ConnectorTableSchema atSchema = metadata.getTableSchema(session,
pinnedHandle, connectorSnapshot);
Review Comment:
Fixed in `0481708aea`. The snapshot-aware Hive delegation now calls the same
capability-reflection helper as ordinary schema delegation. The retained-latest
regression verifies TopN lazy materialization, nested-column pruning, storage
predicate pruning, auto-analyze, and preservation of the sibling
write-generation identity.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -577,7 +581,38 @@ 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());
+ }
+
+ private <T> T readSchemaAuthenticated(Supplier<T> read) {
+ // Cached tables do not cache schema files: exact/latest schema reads
still need plugin UGI and TCCL.
+ try {
+ return context.executeAuthenticated(read::get);
+ } catch (RuntimeException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to read Paimon schema", e);
+ }
+ }
+
+ private long statementSchemaId(PaimonTableHandle handle, Table table) {
+ return statementSchemas.computeIfAbsent(handle,
Review Comment:
Fixed in `0481708aea`. Latest pins carry immutable schema/snapshot content
digests that survive handle reload and metadata-scope replacement. Metadata and
scan preparation validate those files, bypassing the SDK's path-keyed snapshot
cache; aliases retain the first pin. Tests recreate a real Paimon table within
one metadata instance with identical numeric IDs and different rows, then
verify both metadata and scan-handle reload reject it. Another regression
covers recreation between the schema read and pin capture. Validation adds
bounded metadata-file reads and does not alter historical time-travel cache
policy.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -483,33 +488,65 @@ 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();
+ validateSnapshotTable(iceHandle, table, snapshot);
+ 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));
+ if (spec == null) {
+ // Keep the legacy missing-history fallback after checking the
table identity.
+ spec = table.spec();
+ }
+ return buildTableSchema(iceHandle.getTableName(), table, schema, spec,
true);
+ }
+
+ private void validateSnapshotTable(IcebergTableHandle handle, Table table,
ConnectorMvccSnapshot snapshot) {
+ String identity =
snapshot.getProperties().get(TABLE_IDENTITY_PROPERTY);
+ if (identity != null && !identity.equals(tableIdentity(table))) {
+ // Numeric schema/spec IDs can be reused after recreation. Reject
the entire old pin;
+ // replacing only its schema or spec would still mix the new table
with an old data fence.
+ if (latestSnapshotCache != null) {
+
latestSnapshotCache.invalidate(TableIdentifier.of(handle.getDbName(),
handle.getTableName()));
}
+ throw new DorisConnectorException("Iceberg table " +
handle.getDbName() + "." + handle.getTableName()
+ + " identity changed after its snapshot was cached; retry
the statement");
}
- return buildTableSchema(iceHandle.getTableName(), table, schema, true);
+ }
+
+ private static String tableIdentity(Table table) {
+ if (table instanceof HasTableOperations) {
+ TableMetadata metadata = ((HasTableOperations)
table).operations().current();
+ if (metadata.uuid() != null) {
+ return metadata.uuid();
+ }
+ // Legacy V1 metadata may lack a UUID. Only the exact metadata
file can safely reuse its IDs.
+ return "metadata:" +
Objects.requireNonNull(metadata.metadataFileLocation(),
Review Comment:
Fixed in `0481708aea`. UUID-less latest coordinates are evicted and resolved
from the statement's table, including a concurrent cache hit before eviction.
Normal UUID-backed cache hits remain unchanged. A real V1 metadata regression
now performs an ordinary property commit with retained metadata ancestry under
warm-pin/fresh-table conditions and verifies the next statement succeeds; the
separate recreation test still rejects a mismatched retained pin.
--
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]