github-actions[bot] commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r3999300715
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -2131,6 +2134,9 @@ public Optional<ConnectorMvccSnapshot> beginQuerySnapshot(
: loadLatestSnapshotPin(session, iceHandle);
ConnectorMvccSnapshot.Builder snapshot =
ConnectorMvccSnapshot.builder()
.snapshotId(pin.snapshotId).schemaId(pin.schemaId);
+ if (pin.specId >= 0) {
+ snapshot.property(PARTITION_SPEC_ID_PROPERTY,
Integer.toString(pin.specId));
Review Comment:
[P1] Carry this spec ID through the partition-view coordinate too. It is
consumed by `getTableSchema`, but `applySnapshot` drops it because
`IcebergTableHandle` has no spec carrier; `getMvccPartitionView` then reloads a
live table and both derived caches key only on snapshot/schema IDs. A spec-only
add changes neither of those IDs. The new test already caches a one-field
`day(ts)` pin and then adds `id`, but stops at schema assembly: the same pin's
view now sees the live two-field spec and flips RANGE to UNPARTITIONED (or a
warm view cache can pair old one-bound ranges with a new two-column schema).
This is downstream of the earlier schema-builder thread. Please carry/fence
`specId` through the handle, RANGE/LIST view construction, and cache
identities, and extend that test through partition materialization.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -172,6 +172,14 @@ 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);
+ pinnedSchema = toSchemaCacheValue(metadata, session,
Review Comment:
[P1] Build the partition items from this local pinned schema as well. The
style gate, `buildFromRangeView`, and `listLatestPartitions` below all call the
no-argument `getPartitionColumns()`. The pin being constructed is not inserted
into `StatementContext.snapshots` until `loadSnapshot` returns, so those calls
resolve the ambient latest/name-keyed schema rather than `pinnedSchema`. After
schema/spec evolution this can store S1 as the snapshot schema while parsing
one-bound RANGE/LIST values with S2's arity or types, causing skipped
partitions or analysis failures. This is a later consumer than the earlier
Iceberg schema-builder thread. Please derive the partition-column list from
`pinnedSchema` when non-null and pass it through both builders, with an
arity/type-evolution assertion.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -577,7 +584,14 @@ 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 long statementSchemaId(PaimonTableHandle handle, Table table) {
+ return statementSchemaIds.computeIfAbsent(handle,
+ ignored ->
catalogOps.latestSchema(table).map(PaimonCatalogOps.PaimonSchemaSnapshot::schemaId)
Review Comment:
[P1] Keep these schema-file reads inside the authenticated connector scope.
`resolveTable(...)` returns before `statementSchemaId` invokes `latestSchema`;
the catalog-backed implementation calls Paimon `SchemaManager.latest()`, which
lists and reads the schema directory via `FileIO`. A warm latest-snapshot-cache
hit now forces that live read once per statement after the plugin UGI/TCCL
scope closes, and the new eager pinned-schema materialization likewise reaches
the existing `schemaAt` memo miss outside auth. `restoreBoundSchema` then
repeats this with `schemaManager().schema(...)` before scan planning enters
auth, so Kerberized HDFS can fail binding or planning. Please authenticate
exact-schema capture/materialization/restoration and add cache-hit plus
scan-restore auth-scope coverage.
--
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]
