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]

Reply via email to