Gabriel39 commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r4001915269
##########
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,
+ ignored -> readSchemaAuthenticated(() ->
catalogOps.latestSchema(table)))
+ .map(PaimonCatalogOps.PaimonSchemaSnapshot::schemaId)
+ .orElse(-1L);
+ }
+
+ private PaimonCatalogOps.PaimonSchemaSnapshot
schemaForPin(PaimonTableHandle handle, Table table, long schemaId) {
+ // External recreation can reuse a schema ID. Latest pins must retain
the actual statement
+ // schema instead of consulting the name/ID-keyed historical memo from
an earlier table.
+ Optional<PaimonCatalogOps.PaimonSchemaSnapshot> captured =
+ statementSchemas.getOrDefault(handle, Optional.empty());
+ if (captured.isPresent() && captured.get().schemaId() == schemaId) {
+ return captured.get();
+ }
+ return schemaAtMemo.getOrLoad(handle, schemaId,
Review Comment:
This is a valid pre-existing limitation of explicit time travel, but it is
outside the regression scope of this PR.
I checked the implementation immediately before this PR (`ee4a91720f`):
- [Explicit snapshot/timestamp/tag
resolution](https://github.com/apache/doris/blob/ee4a91720f66ec3e2f6023a990e4ebcb9d66fd77/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java#L649-L692)
already resolved the replacement table's numeric IDs. These selector blocks
are unchanged in this PR.
- Both [schema
materialization](https://github.com/apache/doris/blob/ee4a91720f66ec3e2f6023a990e4ebcb9d66fd77/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java#L300-L324)
and [column-handle
resolution](https://github.com/apache/doris/blob/ee4a91720f66ec3e2f6023a990e4ebcb9d66fd77/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java#L1227-L1238)
already called the same cross-query `schemaAtMemo.getOrLoad` lookup.
-
[PaimonSchemaAtMemo](https://github.com/apache/doris/blob/ee4a91720f66ec3e2f6023a990e4ebcb9d66fd77/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonSchemaAtMemo.java#L64-L72)
is byte-for-byte unchanged from that pre-PR revision, including its
name/branch/schema-ID key and lack of TTL.
The new helper preserves that historical fallback (with the authenticated
read wrapper). The regression introduced by this PR was routing ordinary
latest-schema binding into that memo; `54a5294b6c` fixes that by retaining the
actual statement schema and using it for both metadata and column handles.
Explicit snapshot/tag/time reads with no captured latest schema had the
reported behavior before this PR as well.
A generation-aware historical memo or statement-local historical-schema
reads should be handled as a separate fix, including the historical scan
dictionary that also consumes this memo. Resolving this thread as out of scope
for this PR, not as a claim that the pre-existing limitation is fixed.
--
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]
