github-actions[bot] commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r3999524032
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -483,33 +484,34 @@ 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();
- }
- }
- return buildTableSchema(iceHandle.getTableName(), table, schema, true);
+ 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] Fall back when this cached spec ID is absent from the reloaded table.
Production latest pins now always carry the spec ID, but REST vended-credential
catalogs keep that pin cache while deliberately reloading the raw table each
statement. After an external drop/recreate, a warm pin can therefore name spec
1 while the replacement table has only spec 0; this lookup returns null and
`buildTableSchema` later dereferences `spec.isUnpartitioned()`, so every query
can fail until refresh/expiry. The adjacent schema lookup already handles the
same missing-history shape, while the new missing-schema test omits the
production spec property. Please fall back to `table.spec()` (or
invalidate/reload the pin) when the ID is missing and cover a warm-cache
recreate. This is distinct from the existing partition-view thread, where the
historical spec is present but not propagated.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1195,6 +1222,10 @@ OptionalInt backendManifestParallelism(PaimonTableHandle
handle, Table scanTable
*/
// Package-private for direct unit testing (PaimonBackendBoundTableTest).
Table tableForBackend(PaimonTableHandle handle, Table scanTable) {
+ return withBoundSchemaAuthentication(handle, () ->
buildBackendTable(handle, scanTable));
Review Comment:
[P1] Keep the later system-source restorations inside this authentication
scope too. This helper protects `buildBackendTable`, but
`getScanNodeProperties` immediately calls `backendManifestParallelism` and
later prepares `DORIS_SERIALIZED_SYSTEM_SOURCE`; both pass the raw retained
system source through `runtimeSafeSystemSource` after this helper returns. For
a warm `$ro@options` handle bound to S2 with an S1 source, those calls reach
`SchemaManager.schema(S2)` outside `executeAuthenticated`, so Kerberized HDFS
can still fail while building scan properties. The guarded test stops at
`resolveScanTable` and the full properties test uses a context-free provider.
Please reuse the already prepared source or authenticate both later
transformations, and extend the guarded test through `getScanNodeProperties`.
These are distinct later call sites from the earlier auth thread.
--
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]
