From 6614318193597f19bd1fe89eaa0ec8006d1d7f7b Mon Sep 17 00:00:00 2001 From: Wyatt Alt Date: Thu, 17 Sep 2026 20:00:32 +0000 Subject: [PATCH] fix: a metadata update must not rebase over a restore A restore rewinds schema and field metadata with the rest of the manifest. An UpdateConfig carrying schema or field metadata updates rebased over it reinstated metadata the restore discarded, in either commit order, while the same pair is already rejected for Merge and for Overwrite. --- docs/src/format/table/transaction.md | 2 + rust/lance/src/io/commit/conflict_resolver.rs | 48 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/docs/src/format/table/transaction.md b/docs/src/format/table/transaction.md index f7115b7b500..36c97f705ea 100644 --- a/docs/src/format/table/transaction.md +++ b/docs/src/format/table/transaction.md @@ -334,6 +334,7 @@ The Restore operation reverts the table to a previous version. It's generally as other operation. Here are the operations that conflict with Restore: - UpdateMemWalState +- UpdateConfig (only if it updates schema or field metadata, which a restore rewinds) ### ReserveFragments @@ -431,6 +432,7 @@ An UpdateConfig operation only modifies table config and tends to be compatible are the operations that conflict with UpdateConfig: - Overwrite +- Restore (only if the UpdateConfig updates schema or field metadata) - UpdateConfig (only if the two operations modify the same config) ### DataReplacement diff --git a/rust/lance/src/io/commit/conflict_resolver.rs b/rust/lance/src/io/commit/conflict_resolver.rs index 1dd5edde987..a4790ef1195 100644 --- a/rust/lance/src/io/commit/conflict_resolver.rs +++ b/rust/lance/src/io/commit/conflict_resolver.rs @@ -308,6 +308,16 @@ impl<'a> TransactionRebase<'a> { { return Err(self.retryable_conflict_err(other_transaction, other_version)); } + // A restore replaces the manifest with an older one, schema and field + // metadata included. Rebasing a metadata update over it reinstates + // metadata the restore discarded, and re-reading cannot help: the + // update was computed against a schema the table no longer has. + if (matches!(ours, Operation::Restore { .. }) && updates_schema_or_field_metadata(theirs)) + || (updates_schema_or_field_metadata(ours) + && matches!(theirs, Operation::Restore { .. })) + { + return Err(self.incompatible_conflict_err(other_transaction, other_version)); + } let op = &self.transaction.operation; match op { @@ -4221,6 +4231,44 @@ mod tests { ); } + /// A restore rewinds schema and field metadata with the rest of the + /// manifest, so a metadata update must not rebase over it; a plain config + /// upsert carries no metadata and still rebases. + #[rstest::rstest] + #[case::field_metadata(Some(HashMap::from([(0, HashMap::from([("fresh".to_string(), "{}".to_string())]))])), None, true)] + #[case::schema_metadata(None, Some(HashMap::from([("owner".to_string(), "refresh".to_string())])), true)] + #[case::config_only(None, None, false)] + fn test_update_config_conflicts_with_restore( + #[case] field_metadata: Option>>, + #[case] schema_metadata: Option>, + #[case] conflicts: bool, + ) { + let restore = Transaction::new(0, Operation::Restore { version: 1 }, None); + let update = create_update_config_for_test( + Some(HashMap::from([("key".to_string(), "value".to_string())])), + None, + schema_metadata, + field_metadata, + ); + // Either commit order: the metadata the restore discarded must not + // come back, whichever transaction rebases. + for (ours, theirs) in [ + (update.clone(), restore.operation.clone()), + (restore.operation, update), + ] { + let mut rebase = TransactionRebase { + transaction: Transaction::new(0, ours, None), + initial_fragments: HashMap::new(), + modified_fragment_ids: HashSet::new(), + affected_rows: None, + conflicting_frag_reuse_indices: Vec::new(), + conflicting_mem_wal_compacted_sstables: Vec::new(), + }; + let result = rebase.check_txn(&Transaction::new(0, theirs, None), 1); + assert_eq!(result.is_err(), conflicts, "{result:?}"); + } + } + #[test] fn test_mem_wal_install_conflicts_with_merge() { let mem_wal_index = IndexMetadata {