Gabriel39 commented on code in PR #67687:
URL: https://github.com/apache/doris/pull/67687#discussion_r3980103639
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java:
##########
@@ -758,30 +758,45 @@ static boolean isValidRelatedTable(Table table) {
/**
* The cross-query PARTITIONS-scan de-duplication seam (PERF-02): when
{@code cache} is non-null the raw
* partition list is served from / populated into the per-catalog {@link
IcebergPartitionCache} keyed by
- * {@code (id, snapshotId)} — a snapshot is immutable, so the derived
partitions are a pure function of that
- * key and safe to reuse across queries (restoring the legacy
IcebergExternalMetaCache partition-info cache).
+ * {@code (id, snapshotId, schemaId, specId)}. The schema/spec generation
is required because metadata-only
+ * evolution can change the unified partition projection without creating
a snapshot.
* A {@code null} cache (offline unit tests / the no-cache catalog) reads
live every call. The cached list is
- * unmodifiable so a shared entry cannot be mutated by a concurrent
reader; the loader's exception (e.g. the
- * dropped-partition-source-column {@link ValidationException}) propagates
verbatim so callers keep their own
- * degradation, and a failed scan is not cached.
+ * unmodifiable so a shared entry cannot be mutated by a concurrent
reader; loader exceptions propagate
+ * verbatim so callers keep their own degradation, and a failed scan is
not cached.
*/
private static List<IcebergRawPartition> loadRawPartitions(TableIdentifier
id, Table table, long snapshotId,
IcebergPartitionCache cache) {
if (cache == null) {
return loadRawPartitionsUncached(table, snapshotId);
}
- return cache.getOrLoad(new IcebergPartitionCache.Key(id, snapshotId),
+ return cache.getOrLoad(new IcebergPartitionCache.Key(
+ id, snapshotId, table.schema().schemaId(),
table.spec().specId()),
Review Comment:
Fixed in dab9c2ef7a. The derived MVCC and partition-list caches now resolve
the live table before lookup and include table.spec().specId() as an
independent metadata-generation key. Added warm-cache tests across spec-only
commits for both derived views.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java:
##########
@@ -758,30 +758,45 @@ static boolean isValidRelatedTable(Table table) {
/**
* The cross-query PARTITIONS-scan de-duplication seam (PERF-02): when
{@code cache} is non-null the raw
* partition list is served from / populated into the per-catalog {@link
IcebergPartitionCache} keyed by
- * {@code (id, snapshotId)} — a snapshot is immutable, so the derived
partitions are a pure function of that
- * key and safe to reuse across queries (restoring the legacy
IcebergExternalMetaCache partition-info cache).
+ * {@code (id, snapshotId, schemaId, specId)}. The schema/spec generation
is required because metadata-only
+ * evolution can change the unified partition projection without creating
a snapshot.
* A {@code null} cache (offline unit tests / the no-cache catalog) reads
live every call. The cached list is
- * unmodifiable so a shared entry cannot be mutated by a concurrent
reader; the loader's exception (e.g. the
- * dropped-partition-source-column {@link ValidationException}) propagates
verbatim so callers keep their own
- * degradation, and a failed scan is not cached.
+ * unmodifiable so a shared entry cannot be mutated by a concurrent
reader; loader exceptions propagate
+ * verbatim so callers keep their own degradation, and a failed scan is
not cached.
*/
private static List<IcebergRawPartition> loadRawPartitions(TableIdentifier
id, Table table, long snapshotId,
IcebergPartitionCache cache) {
if (cache == null) {
return loadRawPartitionsUncached(table, snapshotId);
}
- return cache.getOrLoad(new IcebergPartitionCache.Key(id, snapshotId),
+ return cache.getOrLoad(new IcebergPartitionCache.Key(
+ id, snapshotId, table.schema().schemaId(),
table.spec().specId()),
() ->
Collections.unmodifiableList(loadRawPartitionsUncached(table, snapshotId)));
}
private static List<IcebergRawPartition> loadRawPartitionsUncached(Table
table, long snapshotId) {
+ StructType unifiedPartitionType = Partitioning.partitionType(table);
+ Map<Integer, Integer> partitionFieldOrdinals = new HashMap<>();
+ for (int i = 0; i < unifiedPartitionType.fields().size(); i++) {
+
partitionFieldOrdinals.put(unifiedPartitionType.fields().get(i).fieldId(), i);
+ }
+ boolean hasUnrepresentableField = table.specs().values().stream()
Review Comment:
Fixed in dab9c2ef7a. Representability is now checked only for the spec
referenced by each live partitions metadata row. A retained unrepresentable
spec with no live files no longer hides valid current partitions, while a live
unrepresentable spec still returns the safe empty fallback. Added the
no-live-old-files test.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -1201,7 +1202,12 @@ private static boolean
isPositionDeletesPartitionColumnRequested(List<ConnectorC
*/
private TableScan buildScan(Table table, IcebergTableHandle handle,
Optional<ConnectorExpression> filter,
ConnectorSession session) {
- TableScan scan = table.newScan();
+ Schema selectedSchema = !handle.isSystemTable() &&
handle.hasSnapshotPin()
+ ? pinnedSchema(table, handle) : table.schema();
+ // Keep the SDK's native Table.newScan implementation unless an actual
historical schema needs the
+ // metadata-only snapshot fix; catalog-specific Table wrappers may
provide their own scan behavior.
+ TableScan scan = !handle.isSystemTable() &&
!selectedSchema.sameSchema(table.schema())
Review Comment:
Fixed in dab9c2ef7a using the explicit-rejection option. Doris now detects
the raw Iceberg 1.11 server-planned table before Kerberos wrapping or scan
construction and reports that client-side scan planning is required. The test
uses a real historical schema after schema evolution and verifies that neither
the native scan nor local metadata is touched.
##########
fe/pom.xml:
##########
@@ -360,7 +360,7 @@ under the License.
<!-- ATTN: avro version must be consistent with Iceberg version -->
<!-- Please modify iceberg.version and avro.version together,
you can find avro version info in iceberg mvn repository -->
- <iceberg.version>1.10.1</iceberg.version>
+ <iceberg.version>1.11.0</iceberg.version>
Review Comment:
Fixed in dab9c2ef7a using the allowed disable path. Server-side planning is
rejected before getScanNodeProperties can read table.io(), so Doris cannot
silently emit BE properties without the scan-scoped credentials that only exist
after planFiles(). Added a scan-properties test that verifies no table
credential access occurs.
--
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]
