github-actions[bot] commented on code in PR #66007:
URL: https://github.com/apache/doris/pull/66007#discussion_r3671201347
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergSnapshotCacheValue.java:
##########
@@ -27,15 +29,27 @@ public class IcebergSnapshotCacheValue {
private final IcebergPartitionInfo partitionInfo;
private final IcebergSnapshot snapshot;
private final Optional<Map<Integer, List<String>>> nameMapping;
+ private final Optional<Table> icebergTable;
Review Comment:
[P1] Freeze metadata rather than the shared live Table
`IcebergExternalMetaCache` stores this same cached `BaseTable` in both the
table entry and the snapshot value. Doris schema/spec changes fetch that object
and commit through its `TableOperations` before invalidating the cache.
Iceberg's `BaseTable.newScan()`, `schema()`, `spec()`, and `currentSnapshot()`
all read `ops.current()`, and a commit marks those operations for refresh. A
query can therefore bind T0/S0, retain this exact object, race with `ALTER
TABLE ... ADD PARTITION FIELD` on the same handle, and have its next scan or
sink call observe T1 while the memoized snapshot/partitions still describe T0.
That recreates the generation split even though object identity did not change.
Please retain an immutable metadata view (or isolated per-statement
operations/base generation) and add a barrier test that advances one shared
`Table` object.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -697,6 +722,20 @@ public TableScan createTableScan() throws UserException {
return icebergTableScan;
}
+ private Table useFrozenTableGeneration(Table currentTable) {
+ Optional<MvccSnapshot> snapshot = getPinnedRelationSnapshot();
+ if (!isSystemTable &&
snapshot.filter(IcebergMvccSnapshot.class::isInstance).isPresent()) {
Review Comment:
[P1] Build snapshot-selectable metadata tables from the pinned base
This excludes every system table from the retained-generation path, although
only the static and `ALL_*` types are intentionally unfenced. `BindRelation`
captures S0 from the base table, while `IcebergSysExternalTable` lazily creates
`FILES`, `DATA_FILES`, `DELETE_FILES`, `MANIFESTS`, `ENTRIES`, and `PARTITIONS`
from a fresh `sourceTable.getIcebergTable()`. After a refresh or snapshot
expiration, execution therefore builds a T1 metadata table and then applies
`useSnapshot(S0)` to it; that can reject the expired ID or derive
schema/spec-dependent metadata from T1 around S0 rows. Please construct
snapshot-selectable metadata tables from the same immutable base generation as
the relation, while keeping only the documented static/`ALL_*` types unfenced,
and add a `t$files` bind/refresh barrier test.
--
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]
