JingsongLi commented on code in PR #752:
URL: https://github.com/apache/paimon-rust/pull/752#discussion_r3900602789
##########
crates/paimon/src/catalog/filesystem.rs:
##########
@@ -539,7 +542,26 @@ impl Catalog for FileSystemCatalog {
full_name: identifier.full_name(),
})?;
- reject_table_type_changes(current.options(), &changes)?;
+ // Only the index-layout option is snapshot-gated, so the listing a
+ // snapshot lookup costs is paid only when a change touches it. Java
defers
+ // the same lookup through a `LazyField`.
+ let touches_snapshot_gated_option = changes.iter().any(|change| {
+ matches!(
+ change,
+ crate::spec::SchemaChange::SetOption { key, .. }
+ | crate::spec::SchemaChange::RemoveOption { key }
+ if key == INDEX_FILE_IN_DATA_FILE_DIR_OPTION
+ )
+ });
+ let has_snapshots = if touches_snapshot_gated_option {
+ SnapshotManager::new(self.file_io.clone(), table_path.clone())
+ .get_latest_snapshot_id()
+ .await?
+ .is_some()
+ } else {
+ false
+ };
+ reject_immutable_option_changes(current.options(), &changes,
has_snapshots)?;
Review Comment:
[P1] Serialize this layout ALTER with first-snapshot commits
This check and `save_table_schema` are independent of snapshot publication.
A writer created before the ALTER has already cached the old
`index-file-in-data-file-dir` layout, so it can write HASH/DV files to the old
directory and then publish the first snapshot after this ALTER succeeds. A
freshly loaded table resolves the manifest file names using the new layout,
making those index files unreachable (and dynamic-bucket reads/writes can fail
with file-not-found). The comment below explicitly documents this interleaving,
but the new tests cover only serial ALTERs. Please serialize schema publication
and snapshot commit through the same lock/CAS, or keep this option immutable
after creation until that ordering exists; add a stale-writer -> ALTER ->
commit -> fresh-table restore regression.
--
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]
