Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/src/format/table/transaction.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This format-spec contract is bundled with its implementation, but docs/src/format/AGENTS.md requires format-spec changes to be isolated for the PMC vote and implementation to follow separately. Please make this PR spec-only and move the resolver/test changes to a follow-up, or first merge a separate spec PR and then rebase this implementation without this doc hunk. That keeps the voted contract independent of implementation details.


### ReserveFragments

Expand Down Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions rust/lance/src/io/commit/conflict_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<HashMap<u32, HashMap<String, String>>>,
#[case] schema_metadata: Option<HashMap<String, String>>,
#[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 {
Expand Down
Loading