diff --git a/protos/transaction/actions.proto b/protos/transaction/actions.proto index 949d8c95ef1..9c2b6dde6d2 100644 --- a/protos/transaction/actions.proto +++ b/protos/transaction/actions.proto @@ -13,7 +13,7 @@ package lance.table; /* * Action-based transactions (Transaction V2) — DRAFT. * - * A `UserOperation` replaces the single legacy `Operation` (see + * A `CompositeOperation` replaces the single legacy `Operation` (see * transaction.proto) with an ordered list of granular `Action`s that commit * atomically as one manifest change. This file is the wire draft only. * @@ -21,7 +21,7 @@ package lance.table; * ------ * The messages and field numbers here are NOT yet a stable contract. Library * support is read-side fail-closed only: a transaction carrying a - * `UserOperation` is rejected on load, and there is no write path. Apply, + * `CompositeOperation` is rejected on load, and there is no write path. Apply, * id translation, and conflict resolution are intentionally absent. * * DESIGN RATIONALE @@ -106,10 +106,10 @@ package lance.table; * id) that may not be committed yet. * * `committed` is an already-assigned id. `local` is a placeholder token minted - * by an `Add*` action earlier in the same `UserOperation`; it resolves to a + * by an `Add*` action earlier in the same `CompositeOperation`; it resolves to a * freshly-allocated committed id at apply, and re-resolves against the target's * counters on merge/rebase. `local` tokens are scoped to a single - * `UserOperation` and must be distinct within it (validated by the writer once + * `CompositeOperation` and must be distinct within it (validated by the writer once * the write path exists). */ message Ref { @@ -123,23 +123,21 @@ message Ref { * A user-facing, composable transaction: an ordered list of user actions that * commit atomically as a single manifest change. */ -message UserOperation { - // Human-readable description, e.g. "INSERT INTO t VALUES (1)". - string description = 1; +message CompositeOperation { // Unique identifier for this operation (matches Transaction.uuid semantics). - string uuid = 2; + string uuid = 1; // The dataset version this operation was planned against. - uint64 read_version = 3; + uint64 read_version = 2; // The ordered list of user actions applied by this operation. - repeated UserAction actions = 4; + repeated UserAction actions = 3; } /* - * A single user-recognizable step within a UserOperation (e.g. "append batch", + * A single user-recognizable step within a CompositeOperation (e.g. "append batch", * "rebuild index"). * * The description keeps the transaction history human-readable. When a range of - * transactions is squashed, each original UserOperation collapses into one + * transactions is squashed, each original CompositeOperation collapses into one * UserAction so the readable sequence survives; the action lists are flattened * when applied to the manifest. */ @@ -173,7 +171,9 @@ message Action { ConfigUpdate config_update = 8; AddOverlays add_overlays = 9; RefreshRowVersionMetadata refresh_row_version_metadata = 10; - UpdateCompactedSsTables update_compacted_sstables = 11; + // Named `update_compacted_ss_tables` rather than `..._sstables` so the + // generated oneof variant matches the message name. + UpdateCompactedSsTables update_compacted_ss_tables = 11; // -- Schema (field-level; no wholesale SetSchema) -- DropField drop_field = 12; AlterField alter_field = 13; @@ -286,8 +286,8 @@ message AddBase { */ message TombstoneFieldData { Ref fragment = 1; - // Committed field ids whose current backing is tombstoned. - repeated uint64 field_ids = 2; + // The fields whose current backing is tombstoned. + repeated Ref field_ids = 2; // Data-change marker; see AddFragment.data_change. optional bool data_change = 3; } @@ -376,7 +376,7 @@ message UpdateCompactedSsTables { * no live fields). References an existing committed field id. */ message DropField { - uint64 field = 1; + Ref field = 1; } /* @@ -389,7 +389,7 @@ message DropField { * AddDataFile to rewrite the data. New facets are added as optional fields. */ message AlterField { - uint64 field = 1; + Ref field = 1; optional string name = 2; // New Arrow logical type (see Field.logical_type). The cast. optional string logical_type = 3; @@ -411,9 +411,15 @@ message AddIndexSegment { optional int32 index_version = 5; /* * Fragments covered by this segment (resolved to a fragment_bitmap at apply, - * and remapped under fragment relocation). Committed or same-op Local. + * and remapped under fragment relocation). + * + * Absent means the coverage is unknown, which is what the system indices + * (MemWAL, fragment reuse) carry and what pre-bitmap segments read back as. + * That is a different statement from a present-but-empty list, which says the + * segment covers no fragment at all -- the query path serves a segment of + * unknown coverage and skips one that covers nothing. */ - repeated Ref covered_fragments = 6; + optional FragmentCoverage covered_fragments = 6; /* * Index files with sizes, when produced by the writer (e.g. a compaction that * rewrites the segment). Empty when unavailable. @@ -422,6 +428,37 @@ message AddIndexSegment { // Data-change marker; see AddFragment.data_change. false marks a segment // rebuild/compaction that does not reflect any change to the indexed data. optional bool data_change = 8; + /* + * The base path this segment's files live under, for a segment imported from + * another dataset. Committed, or Local for a base minted by an AddBase in the + * same operation. Absent => the dataset's own index directory. + */ + optional Ref base = 9; + /* + * When this segment was built, in milliseconds since the Unix epoch. Absent + * for a segment whose build time was not recorded. + */ + optional uint64 created_at = 10; + /* + * The dataset version whose data this segment reflects. + * + * This is a correctness gate, not provenance: an overlay committed at or + * before it is treated as already folded into the index. A segment merged + * from several older ones reflects only as much as its oldest input, so this + * is genuinely below the version the operation reads and cannot be derived + * from it. Absent means the version the operation reads, which is what a + * freshly built segment reflects. It may never exceed that version. + */ + optional uint64 dataset_version = 11; +} + +/* + * A list of fragments, wrapped so that "no coverage recorded" is distinguishable + * from "covers nothing" (a bare `repeated` field cannot tell them apart). + */ +message FragmentCoverage { + // Committed, or Local for a fragment minted in the same operation. + repeated Ref fragments = 1; } /* diff --git a/protos/transaction/transaction.proto b/protos/transaction/transaction.proto index 6da49fd8668..09ba8d41e37 100644 --- a/protos/transaction/transaction.proto +++ b/protos/transaction/transaction.proto @@ -343,7 +343,7 @@ message Transaction { DataOverlay data_overlay = 115; // Action-based transaction (Transaction V2). See actions.proto. // DRAFT: currently rejected on load; no write path. - UserOperation user_operation = 116; + CompositeOperation composite_operation = 116; } // Fields 200/202 (`blob_append` / `blob_overwrite`) previously represented blob dataset ops. diff --git a/rust/lance-table/src/format/fragment.rs b/rust/lance-table/src/format/fragment.rs index 149a9b44fc6..bf49d742fee 100644 --- a/rust/lance-table/src/format/fragment.rs +++ b/rust/lance-table/src/format/fragment.rs @@ -455,6 +455,22 @@ impl TryFrom for DeletionFile { } } +impl From<&DeletionFile> for pb::DeletionFile { + fn from(value: &DeletionFile) -> Self { + let file_type = match value.file_type { + DeletionFileType::Array => pb::deletion_file::DeletionFileType::ArrowArray, + DeletionFileType::Bitmap => pb::deletion_file::DeletionFileType::Bitmap, + }; + Self { + read_version: value.read_version, + id: value.id, + file_type: file_type.into(), + num_deleted_rows: value.num_deleted_rows.unwrap_or_default() as u64, + base_id: value.base_id, + } + } +} + /// Data fragment. /// /// A fragment is a set of files which represent the different columns of the same rows. @@ -716,19 +732,7 @@ impl TryFrom for Fragment { impl From<&Fragment> for pb::DataFragment { fn from(f: &Fragment) -> Self { - let deletion_file = f.deletion_file.as_ref().map(|f| { - let file_type = match f.file_type { - DeletionFileType::Array => pb::deletion_file::DeletionFileType::ArrowArray, - DeletionFileType::Bitmap => pb::deletion_file::DeletionFileType::Bitmap, - }; - pb::DeletionFile { - read_version: f.read_version, - id: f.id, - file_type: file_type.into(), - num_deleted_rows: f.num_deleted_rows.unwrap_or_default() as u64, - base_id: f.base_id, - } - }); + let deletion_file = f.deletion_file.as_ref().map(pb::DeletionFile::from); let row_id_sequence = f.row_id_meta.as_ref().map(|m| match m { RowIdMeta::Inline(data) => { diff --git a/rust/lance-table/src/transaction.rs b/rust/lance-table/src/transaction.rs index 74fba1b33ba..145886ca9f0 100644 --- a/rust/lance-table/src/transaction.rs +++ b/rust/lance-table/src/transaction.rs @@ -17,6 +17,7 @@ //! ```text //! builder Transaction: an operation plus the version it was based on //! operation the vocabulary of changes an operation can describe +//! action the finer-grained Transaction V2 vocabulary (draft) //! update_map incremental edits to the manifest's string maps //! validate pre-commit checks against the manifest being replaced //! manifest_build applying an operation to produce the next manifest @@ -26,6 +27,7 @@ //! proto the persisted protobuf encoding of all of the above //! ``` +pub mod action; mod builder; mod conflicts; mod index_maintenance; diff --git a/rust/lance-table/src/transaction/action.rs b/rust/lance-table/src/transaction/action.rs new file mode 100644 index 00000000000..f4a0b4543b3 --- /dev/null +++ b/rust/lance-table/src/transaction/action.rs @@ -0,0 +1,324 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! The action vocabulary of Transaction V2. +//! +//! Where an [`Operation`](super::Operation) names one whole change and carries a +//! post-image of the parts of the manifest it touches, a [`CompositeOperation`] is an +//! ordered list of [`Action`]s, each recording a single *delta*. Composing +//! several changes into one atomic commit and replaying a change onto a +//! different version both fall out of that, with no per-operation logic. +//! +//! The wire format and the reasoning behind it live in +//! `protos/transaction/actions.proto`; the two definitions must stay in step. +//! Every action the draft defines is implemented here, so the two vocabularies +//! now coincide -- an action a newer Lance writes is rejected on load rather +//! than skipped. +//! +//! Each action lives in its own module and owns everything about itself: its +//! definition, how it is applied, which coordinates it writes, and its wire +//! encoding. This module holds the shared vocabulary and dispatches to them. +//! +//! ```text +//! action the vocabulary and the dispatch (this module) +//! action:: one action, end to end +//! action::apply the working state an action set is applied against +//! action::footprint comparing two action sets for conflicts +//! action::proto the envelope around the per-action encodings +//! action::translate lowering a named operation into actions +//! ``` +//! +//! # Stability +//! +//! Transaction V2 is a pre-vote draft. Nothing in this module is a compatibility +//! contract, and a transaction carrying a [`CompositeOperation`] is rejected outright +//! by libraries that predate it. + +/// The action vocabulary, as one list. +/// +/// Every per-variant `match` over an [`Action`] -- the enum itself, its +/// forwarding methods, and both directions of its wire encoding -- is generated +/// from this, so an action is added by writing its module and adding one name +/// here. A module that does not supply the full set of methods fails to +/// compile. +/// +/// The variant name doubles as the protobuf `oneof` variant name and as the +/// action's name in errors and logs, so the three cannot drift apart. +/// +/// This is defined ahead of the modules below so it is in scope for all of +/// them; `macro_rules!` visibility runs from the definition to the end of the +/// enclosing module, children included. +macro_rules! for_each_action { + ($emit:ident) => { + $emit! { + AddFragment, + AddDataFile, + AddField, + AddBase, + TombstoneFieldData, + RemoveFragment, + SetDeletionFile, + AddOverlays, + RefreshRowVersionMetadata, + UpdateCompactedSsTables, + AlterField, + DropField, + AddIndexSegment, + RemoveIndexSegment, + AdjustIndexCoverage, + ReserveFragmentIds, + ResetTable, + ConfigUpdate, + AssertUniqueKeys, + } + }; +} + +mod add_base; +mod add_data_file; +mod add_field; +mod add_fragment; +mod add_index_segment; +mod add_overlays; +mod adjust_index_coverage; +mod alter_field; +mod apply; +mod assert_unique_keys; +mod config_update; +mod drop_field; +mod footprint; +mod proto; +mod refresh_row_version_metadata; +mod remove_fragment; +mod remove_index_segment; +mod reserve_fragment_ids; +mod reset_table; +mod set_deletion_file; +mod tombstone_field_data; +mod translate; +mod update_compacted_sstables; + +#[cfg(test)] +mod test_support; + +pub use add_base::AddBase; +pub use add_data_file::AddDataFile; +pub use add_field::AddField; +pub use add_fragment::AddFragment; +pub use add_index_segment::AddIndexSegment; +pub use add_overlays::AddOverlays; +pub use adjust_index_coverage::AdjustIndexCoverage; +pub use alter_field::AlterField; +pub use assert_unique_keys::AssertUniqueKeys; +pub use config_update::{ConfigUpdate, FieldMetadataUpdate}; +pub use drop_field::DropField; +pub use footprint::{ConfigMap, Coordinate, Footprint}; +pub use refresh_row_version_metadata::RefreshRowVersionMetadata; +pub use remove_fragment::RemoveFragment; +pub use remove_index_segment::RemoveIndexSegment; +pub use reserve_fragment_ids::ReserveFragmentIds; +pub use reset_table::ResetTable; +pub use set_deletion_file::SetDeletionFile; +pub use tombstone_field_data::TombstoneFieldData; +pub use update_compacted_sstables::UpdateCompactedSsTables; + +use apply::ApplyState; +use lance_core::Result; +use lance_core::deepsize::DeepSizeOf; + +/// A reference to a counter-allocated identifier -- a field id, fragment id, or +/// base id -- that may not have been assigned yet. +/// +/// [`Ref::Committed`] is a concrete id that already exists in the manifest. +/// [`Ref::Local`] is a placeholder token minted by an `Add*` action earlier in +/// the same [`CompositeOperation`]; it resolves to a freshly-allocated id at apply, +/// and re-resolves against the target's counters when the operation is replayed +/// onto a newer version. That re-resolution is what lets two independent +/// `AddField`s on divergent branches become two distinct fields rather than a +/// collision. +/// +/// Local tokens are scoped to one [`CompositeOperation`] and must be distinct within +/// it. The three id spaces do not share a token namespace: a fragment token 0 +/// and a field token 0 are unrelated. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, DeepSizeOf)] +pub enum Ref { + Committed(u64), + Local(u32), +} + +impl Ref { + /// The committed id, or `None` if this is an unresolved local token. + pub fn committed(&self) -> Option { + match self { + Self::Committed(id) => Some(*id), + Self::Local(_) => None, + } + } + + /// The local token, or `None` if this reference is already committed. + pub fn local(&self) -> Option { + match self { + Self::Local(token) => Some(*token), + Self::Committed(_) => None, + } + } +} + +/// A composable transaction: an ordered list of user actions that commit +/// atomically as a single manifest change. +/// +/// The `uuid` and `read_version` carried on the wire mirror the enclosing +/// [`Transaction`](super::Transaction) and are filled in from it, so they are +/// not repeated here. +#[derive(Debug, Clone, PartialEq, DeepSizeOf, Default)] +pub struct CompositeOperation { + /// The ordered steps this operation applies. + pub actions: Vec, +} + +impl CompositeOperation { + pub fn new(actions: Vec) -> Self { + Self { actions } + } + + /// Every action in every step, in application order. + pub fn iter_actions(&self) -> impl Iterator { + self.actions.iter().flat_map(|step| step.actions.iter()) + } +} + +/// A single user-recognizable step within a [`CompositeOperation`], e.g. "append +/// batch" or "rebuild index". +/// +/// The description keeps transaction history readable: when a range of versions +/// is squashed, each original operation collapses into one step, so the sequence +/// a user performed survives even though the deltas are flattened when applied. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct UserAction { + pub description: String, + pub actions: Vec, +} + +impl UserAction { + pub fn new(description: impl Into, actions: Vec) -> Self { + Self { + description: description.into(), + actions, + } + } +} + +macro_rules! define_action { + ($($variant:ident,)*) => { + /// A single granular change to the manifest, or an assertion about the + /// version it lands on. + /// + /// Each variant is defined, applied, and encoded in the module named + /// after it, and appears here only because it is listed in + /// `for_each_action!`. + #[derive(Debug, Clone, PartialEq, DeepSizeOf)] + pub enum Action { + $($variant($variant),)* + } + + impl Action { + pub fn name(&self) -> &'static str { + match self { + $(Self::$variant(_) => stringify!($variant),)* + } + } + + /// Whether this action changes the data a reader would see, as + /// opposed to rearranging how it is stored (compaction, a segment + /// rebuild). + /// + /// CDC and streaming consumers use this to skip commits that cannot + /// have changed any row's value. + pub fn is_data_change(&self) -> bool { + match self { + $(Self::$variant(action) => action.is_data_change(),)* + } + } + + /// Fold this action into the state the next manifest is built from. + fn apply(&self, state: &mut ApplyState) -> Result<()> { + match self { + $(Self::$variant(action) => action.apply(state),)* + } + } + + /// Record the coordinates this action writes. + fn footprint(&self, footprint: &mut Footprint) { + match self { + $(Self::$variant(action) => action.footprint(footprint),)* + } + } + } + }; +} + +for_each_action!(define_action); + +impl std::fmt::Display for Action { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.name()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ref_accessors() { + assert_eq!(Ref::Committed(7).committed(), Some(7)); + assert_eq!(Ref::Committed(7).local(), None); + assert_eq!(Ref::Local(2).local(), Some(2)); + assert_eq!(Ref::Local(2).committed(), None); + } + + #[test] + fn test_data_change_defaults_by_action_kind() { + let alter = Action::AlterField(AlterField { + field: Ref::Committed(1), + name: Some("renamed".into()), + logical_type: None, + nullable: None, + }); + assert!(!alter.is_data_change(), "a rename changes no row values"); + + let remove = Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(3), + data_change: true, + }); + assert!(remove.is_data_change()); + } + + #[test] + fn test_iter_actions_flattens_steps_in_order() { + let operation = CompositeOperation::new(vec![ + UserAction::new( + "first", + vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(1), + data_change: true, + })], + ), + UserAction::new( + "second", + vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(2), + data_change: true, + })], + ), + ]); + let fragments = operation + .iter_actions() + .map(|action| match action { + Action::RemoveFragment(remove) => remove.fragment, + other => panic!("unexpected action {other}"), + }) + .collect::>(); + assert_eq!(fragments, vec![Ref::Committed(1), Ref::Committed(2)]); + } +} diff --git a/rust/lance-table/src/transaction/action/add_base.rs b/rust/lance-table/src/transaction/action/add_base.rs new file mode 100644 index 00000000000..98df7506d7a --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_base.rs @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Mint a new base path. + +use super::apply::ApplyState; +use super::proto::required; +use super::{Coordinate, Footprint}; +use crate::format::{BasePath, pb}; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Mint a new base path. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AddBase { + /// Token standing in for the base id until it is allocated at apply. + pub local: u32, + /// The base path. Its `id` is ignored and stamped in at apply. + pub base: BasePath, +} + +impl AddBase { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let id = state.mint_base(self.local)?; + + let conflicting = state + .bases() + .find(|base| base.name == self.base.name || base.path == self.base.path); + if let Some(conflicting) = conflicting { + return Err(Error::invalid_input(format!( + "Conflict detected: Base path with name '{:?}' or path '{}' already exists. \ + Existing: name='{:?}', path='{}'", + self.base.name, self.base.path, conflicting.name, conflicting.path + ))); + } + + let mut base = self.base.clone(); + base.id = id; + state.push_base(base); + Ok(()) + } + + /// A base path is a location, not data. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// The base id is minted, but the name and location are not: the manifest + /// requires both to be unique, so two operations claiming either collide. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::BaseName(self.base.name.clone())); + footprint.add(Coordinate::BaseLocation(self.base.path.clone())); + } +} + +impl From<&AddBase> for pb::AddBase { + fn from(value: &AddBase) -> Self { + Self { + local: value.local, + base: Some(pb::BasePath::from(value.base.clone())), + } + } +} + +impl TryFrom for AddBase { + type Error = Error; + + fn try_from(message: pb::AddBase) -> Result { + Ok(Self { + local: message.local, + base: BasePath::from(required(message.base, "AddBase.base")?), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::Action; + use crate::transaction::action::test_support::apply; + use crate::transaction::test_support::sample_manifest; + + #[test] + fn test_add_base_mints_an_id_and_rejects_duplicates() { + let manifest = sample_manifest(); + let next = apply( + &manifest, + vec![Action::AddBase(AddBase { + local: 0, + base: BasePath::new(0, "s3://bucket/a".into(), Some("a".into()), false), + })], + ) + .unwrap(); + assert_eq!(next.base_paths.len(), 1); + assert_eq!(next.base_paths[&1].path, "s3://bucket/a"); + + let error = apply( + &next, + vec![Action::AddBase(AddBase { + local: 0, + base: BasePath::new(0, "s3://bucket/a".into(), Some("other".into()), false), + })], + ) + .unwrap_err(); + assert!( + error.to_string().contains("already exists"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_two_add_bases_in_one_operation_see_each_other() { + let manifest = sample_manifest(); + let error = apply( + &manifest, + vec![ + Action::AddBase(AddBase { + local: 0, + base: BasePath::new(0, "s3://bucket/a".into(), Some("a".into()), false), + }), + Action::AddBase(AddBase { + local: 1, + base: BasePath::new(0, "s3://bucket/b".into(), Some("a".into()), false), + }), + ], + ) + .unwrap_err(); + assert!( + error.to_string().contains("already exists"), + "unexpected error: {error}" + ); + } +} diff --git a/rust/lance-table/src/transaction/action/add_data_file.rs b/rust/lance-table/src/transaction/action/add_data_file.rs new file mode 100644 index 00000000000..6fe28c7270c --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_data_file.rs @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Add a data file to a fragment. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Footprint, Ref}; +use crate::format::{DataFile, pb}; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Add a data file to a fragment. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AddDataFile { + /// The fragment to add the file to: committed, or a fragment minted earlier + /// in the same operation. + pub fragment: Ref, + /// The file. Its `fields` are placeholders and are stamped in at apply from + /// `field_ids`, which is the authority for the column -> field mapping. + pub file: DataFile, + /// One entry per column in `file`, in column order. + pub field_ids: Vec, + /// See [`AddFragment::data_change`](super::AddFragment::data_change). + pub data_change: bool, +} + +impl AddDataFile { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let fragment_id = state.resolve_fragment(self.fragment)?; + let field_ids = self + .field_ids + .iter() + .map(|field| state.resolve_field(*field)) + .collect::>>()?; + + let mut file = self.file.clone(); + if !file.column_indices.is_empty() && file.column_indices.len() != field_ids.len() { + return Err(Error::invalid_input(format!( + "AddDataFile for fragment {fragment_id} lists {} field ids but the file has {} \ + columns", + field_ids.len(), + file.column_indices.len() + ))); + } + file.fields = field_ids.into(); + + state + .fragment_mut(fragment_id, "AddDataFile")? + .files + .push(file); + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// The data of every committed field the file backs, in the fragment it is + /// attached to. A file backing only minted fields writes nothing. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add_field_data(self.fragment, self.field_ids.iter().copied()); + } +} + +impl From<&AddDataFile> for pb::AddDataFile { + fn from(value: &AddDataFile) -> Self { + Self { + fragment: Some(value.fragment.into()), + file: Some(pb::DataFile::from(&value.file)), + field_ids: value.field_ids.iter().map(|id| (*id).into()).collect(), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for AddDataFile { + type Error = Error; + + fn try_from(message: pb::AddDataFile) -> Result { + Ok(Self { + fragment: required(message.fragment, "AddDataFile.fragment")?.try_into()?, + file: DataFile::try_from(required(message.file, "AddDataFile.file")?)?, + field_ids: message + .field_ids + .into_iter() + .map(Ref::try_from) + .collect::>>()?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::Action; + use crate::transaction::action::test_support::apply; + use crate::transaction::test_support::sample_manifest; + use lance_file::version::ConcreteFileVersion; + + #[test] + fn test_add_data_file_rejects_an_unbound_local_token() { + let manifest = sample_manifest(); + let error = apply( + &manifest, + vec![Action::AddDataFile(AddDataFile { + fragment: Ref::Local(3), + file: DataFile::new_unstarted("data/x.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Committed(0)], + data_change: true, + })], + ) + .unwrap_err(); + assert!( + error.to_string().contains("local fragment token 3"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_add_data_file_rejects_a_column_count_mismatch() { + let manifest = sample_manifest(); + let mut file = DataFile::new_unstarted("data/x.lance", ConcreteFileVersion::V2_0); + file.column_indices = vec![0, 1].into(); + + let error = apply( + &manifest, + vec![Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(0), + file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("1 field ids"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_add_data_file_rejects_a_missing_fragment() { + let manifest = sample_manifest(); + let error = apply( + &manifest, + vec![Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(7), + file: DataFile::new_unstarted("data/x.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Committed(0)], + data_change: true, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("fragment 7"), "{error}"); + } +} diff --git a/rust/lance-table/src/transaction/action/add_field.rs b/rust/lance-table/src/transaction/action/add_field.rs new file mode 100644 index 00000000000..0fe9b6fcf24 --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_field.rs @@ -0,0 +1,238 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Mint a new schema field. + +use super::apply::ApplyState; +use super::proto::required; +use super::{Footprint, Ref}; +use crate::format::pb; +use lance_core::datatypes::Field; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Mint a new schema field. +/// +/// A nested column that introduces several fields is several ordered +/// `AddField`s -- parent first, each child naming its parent's local token. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AddField { + /// Token standing in for the field id until it is allocated at apply. + pub local: u32, + /// The parent field, or `None` for a top-level column. + pub parent: Option, + /// The field definition. Its `id`, `parent_id`, and `children` are ignored: + /// `local` and `parent` carry that structure, and each child is its own + /// action. + pub def: Field, +} + +impl AddField { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let id = state.mint_field(self.local)?; + let parent_id = self + .parent + .map(|parent| state.resolve_field(parent)) + .transpose()?; + + // The definition's own id, parent id, and children are ignored: the + // minted id and the parent reference carry that structure, and each + // child column arrives as its own action. + let field = Field { + id, + parent_id: parent_id.unwrap_or(-1), + children: Vec::new(), + ..self.def.clone() + }; + + match parent_id { + None => state.schema_mut().fields.push(field), + Some(parent_id) => { + let parent = state + .schema_mut() + .field_by_id_mut(parent_id) + .ok_or_else(|| { + Error::invalid_input(format!( + "AddField names parent field {parent_id}, which does not exist" + )) + })?; + parent.children.push(field); + } + } + Ok(()) + } + + /// A new field starts empty; an + /// [`AddDataFile`](super::AddDataFile) is what gives it values. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// Nothing: the field does not exist in the read version. Attaching it + /// under a committed parent does not rewrite the parent's definition. + pub(super) fn footprint(&self, _footprint: &mut Footprint) {} +} + +impl From<&AddField> for pb::AddField { + fn from(value: &AddField) -> Self { + Self { + local: value.local, + parent: value.parent.map(Into::into), + def: Some(lance_file::format::pb::Field::from(&value.def)), + } + } +} + +impl TryFrom for AddField { + type Error = Error; + + fn try_from(message: pb::AddField) -> Result { + Ok(Self { + local: message.local, + parent: message.parent.map(Ref::try_from).transpose()?, + def: Field::from(&required(message.def, "AddField.def")?), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::action::test_support::{added_field, apply}; + use crate::transaction::action::{Action, AddDataFile}; + use crate::transaction::test_support::sample_manifest; + use arrow_schema::{DataType, Field as ArrowField}; + use lance_file::version::ConcreteFileVersion; + + #[test] + fn test_two_add_fields_mint_distinct_ids() { + let manifest = sample_manifest(); + let next = apply( + &manifest, + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("a"), + }), + Action::AddField(AddField { + local: 1, + parent: None, + def: added_field("b"), + }), + ], + ) + .unwrap(); + + let ids = next + .schema + .fields + .iter() + .map(|f| (f.name.as_str(), f.id)) + .collect::>(); + assert_eq!(ids, vec![("id", 0), ("a", 1), ("b", 2)]); + } + + #[test] + fn test_add_field_then_add_its_data_file() { + // The add-column shape: mint the field, then write the file that backs + // it, naming the field by the token the mint has not resolved yet. + let manifest = sample_manifest(); + let next = apply( + &manifest, + vec![ + Action::AddField(AddField { + local: 7, + parent: None, + def: added_field("added"), + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(0), + file: DataFile::new_unstarted("data/added.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Local(7)], + data_change: true, + }), + ], + ) + .unwrap(); + + let field_id = next.schema.field("added").unwrap().id; + assert_eq!(field_id, 1); + let fragment = next.fragments.iter().find(|f| f.id == 0).unwrap(); + assert_eq!(fragment.files.last().unwrap().fields.as_ref(), &[field_id]); + } + + #[test] + fn test_add_field_under_a_parent() { + let manifest = sample_manifest(); + let next = apply( + &manifest, + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: Field::try_from(ArrowField::new( + "nested", + DataType::Struct(Default::default()), + true, + )) + .unwrap(), + }), + Action::AddField(AddField { + local: 1, + parent: Some(Ref::Local(0)), + def: added_field("child"), + }), + ], + ) + .unwrap(); + + let parent = next.schema.field("nested").unwrap(); + assert_eq!(parent.children.len(), 1); + assert_eq!(parent.children[0].name, "child"); + assert_eq!(parent.children[0].parent_id, parent.id); + } + + #[test] + fn test_add_field_rejects_a_missing_parent() { + let manifest = sample_manifest(); + let error = apply( + &manifest, + vec![Action::AddField(AddField { + local: 0, + parent: Some(Ref::Committed(7)), + def: added_field("orphan"), + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("parent field 7"), "{error}"); + } + + #[test] + fn test_duplicate_local_token_is_rejected() { + let manifest = sample_manifest(); + let error = apply( + &manifest, + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("a"), + }), + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("b"), + }), + ], + ) + .unwrap_err(); + assert!( + error.to_string().contains("minted more than once"), + "unexpected error: {error}" + ); + } +} diff --git a/rust/lance-table/src/transaction/action/add_fragment.rs b/rust/lance-table/src/transaction/action/add_fragment.rs new file mode 100644 index 00000000000..c158bc4a512 --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_fragment.rs @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Mint a new, empty fragment. + +use super::Footprint; +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire}; +use crate::format::{ExternalFile, Fragment, RowIdMeta, pb}; +use crate::rowids::version::RowDatasetVersionMeta; +use lance_core::Result; +use lance_core::deepsize::DeepSizeOf; + +/// Mint a new, empty fragment. +/// +/// Its data files arrive via [`AddDataFile`](super::AddDataFile) actions naming +/// this fragment's local token. A freshly-minted fragment has no deletion +/// vector: it has no committed rows to delete yet. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AddFragment { + /// Token standing in for the fragment id until it is allocated at apply. + pub local: u32, + /// Physical rows in the fragment, including rows later tombstoned. + pub physical_rows: u64, + /// Stable row id sequence. `None` on datasets without stable row ids, and + /// on datasets that have them but where the ids are assigned at apply. + pub row_id_meta: Option, + /// Per-row version metadata, carried exactly as on + /// [`Fragment`](crate::format::Fragment). + /// + /// `None` means "stamp at apply", which fills both with a uniform sequence + /// at the commit version. That is right for an append, but not for the two + /// producers whose rows carry versions from before this commit, so they set + /// the fields explicitly: an update resolves each row's `created_at` from + /// the fragment the row came from, and a compaction rechunks both sequences + /// off the fragments it merged, since moving a row does not update it. + pub last_updated_at_version_meta: Option, + pub created_at_version_meta: Option, + /// `false` marks a pure rearrangement, e.g. a compaction rewrite. + pub data_change: bool, +} + +impl AddFragment { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let id = state.mint_fragment(self.local)?; + state.push_fragment(Fragment { + id, + files: Vec::new(), + overlays: Vec::new(), + deletion_file: None, + row_id_meta: self.row_id_meta.clone(), + physical_rows: Some(self.physical_rows as usize), + last_updated_at_version_meta: self.last_updated_at_version_meta.clone(), + created_at_version_meta: self.created_at_version_meta.clone(), + }); + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// No coordinate: the fragment does not exist in the read version, so no + /// concurrent writer can be naming it. It does record that rows arrive, + /// which is the one thing about a minted fragment a concurrent + /// [`AssertUniqueKeys`](super::AssertUniqueKeys) has to know. A fragment + /// that is not a data change holds rows that were already in the dataset -- + /// a compaction rewrite -- and brings in no new key. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + if self.data_change { + footprint.insert_rows(); + } + } +} + +fn external_file_to_wire(file: &ExternalFile) -> pb::ExternalFile { + pb::ExternalFile { + path: file.path.clone(), + offset: file.offset, + size: file.size, + } +} + +fn external_file_from_wire(file: pb::ExternalFile) -> ExternalFile { + ExternalFile { + path: file.path, + offset: file.offset, + size: file.size, + } +} + +impl From<&AddFragment> for pb::AddFragment { + fn from(value: &AddFragment) -> Self { + Self { + local: value.local, + physical_rows: value.physical_rows, + row_id_sequence: value.row_id_meta.as_ref().map(|meta| match meta { + RowIdMeta::Inline(data) => { + pb::add_fragment::RowIdSequence::InlineRowIds(data.to_vec()) + } + RowIdMeta::External(file) => { + pb::add_fragment::RowIdSequence::ExternalRowIds(external_file_to_wire(file)) + } + }), + last_updated_at_version_sequence: value + .last_updated_at_version_meta + .as_ref() + .map(|meta| match meta { + RowDatasetVersionMeta::Inline(data) => { + pb::add_fragment::LastUpdatedAtVersionSequence::InlineLastUpdatedAtVersions( + data.to_vec(), + ) + } + RowDatasetVersionMeta::External(file) => { + pb::add_fragment::LastUpdatedAtVersionSequence::ExternalLastUpdatedAtVersions( + external_file_to_wire(file), + ) + } + }), + created_at_version_sequence: value + .created_at_version_meta + .as_ref() + .map(|meta| match meta { + RowDatasetVersionMeta::Inline(data) => { + pb::add_fragment::CreatedAtVersionSequence::InlineCreatedAtVersions( + data.to_vec(), + ) + } + RowDatasetVersionMeta::External(file) => { + pb::add_fragment::CreatedAtVersionSequence::ExternalCreatedAtVersions( + external_file_to_wire(file), + ) + } + }), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for AddFragment { + type Error = lance_core::Error; + + fn try_from(message: pb::AddFragment) -> Result { + Ok(Self { + local: message.local, + physical_rows: message.physical_rows, + row_id_meta: message.row_id_sequence.map(|sequence| match sequence { + pb::add_fragment::RowIdSequence::InlineRowIds(data) => { + RowIdMeta::Inline(data.into()) + } + pb::add_fragment::RowIdSequence::ExternalRowIds(file) => { + RowIdMeta::External(external_file_from_wire(file)) + } + }), + last_updated_at_version_meta: message.last_updated_at_version_sequence.map( + |sequence| { + match sequence { + pb::add_fragment::LastUpdatedAtVersionSequence::InlineLastUpdatedAtVersions( + data, + ) => RowDatasetVersionMeta::Inline(data.into()), + pb::add_fragment::LastUpdatedAtVersionSequence::ExternalLastUpdatedAtVersions( + file, + ) => RowDatasetVersionMeta::External(external_file_from_wire(file)), + } + }, + ), + created_at_version_meta: message.created_at_version_sequence.map(|sequence| { + match sequence { + pb::add_fragment::CreatedAtVersionSequence::InlineCreatedAtVersions(data) => { + RowDatasetVersionMeta::Inline(data.into()) + } + pb::add_fragment::CreatedAtVersionSequence::ExternalCreatedAtVersions(file) => { + RowDatasetVersionMeta::External(external_file_from_wire(file)) + } + } + }), + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::action::test_support::apply; + use crate::transaction::action::{Action, AddDataFile, Ref}; + use crate::transaction::test_support::sample_manifest; + use lance_file::version::ConcreteFileVersion; + + #[test] + fn test_add_fragment_and_data_file_mint_ids() { + let manifest = sample_manifest(); + let next = apply( + &manifest, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: DataFile::new_unstarted("data/new.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + ], + ) + .unwrap(); + + // sample_manifest already holds fragment 0, so the mint lands on 1. + assert_eq!( + next.fragments.iter().map(|f| f.id).collect::>(), + vec![0, 1] + ); + let minted = next.fragments.iter().find(|f| f.id == 1).unwrap(); + assert_eq!(minted.physical_rows, Some(10)); + assert_eq!(minted.files.len(), 1); + // The file's field list is stamped in from the action's refs. + assert_eq!(minted.files[0].fields.as_ref(), &[0]); + assert_eq!(next.max_fragment_id(), Some(1)); + } +} diff --git a/rust/lance-table/src/transaction/action/add_index_segment.rs b/rust/lance-table/src/transaction/action/add_index_segment.rs new file mode 100644 index 00000000000..b14755e2ad7 --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_index_segment.rs @@ -0,0 +1,498 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Add an index segment. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Coordinate, Footprint, Ref}; +use crate::format::{IndexFile, IndexMetadata, pb}; +use lance_core::deepsize::{Context, DeepSizeOf}; +use lance_core::{Error, Result}; +use roaring::RoaringBitmap; +use std::sync::Arc; +use uuid::Uuid; + +/// Add an index segment. +/// +/// The format has no first-class "index" apart from its segments: a logical +/// index is the set of segments sharing a `name`, and a brand-new index is +/// written as its first segment. So this one action covers both creating an +/// index and extending an existing one -- the difference is only whether any +/// segment already carries the name. +/// +/// The segment's `uuid` is chosen by the writer rather than minted from a +/// counter, so unlike a fragment or a field it needs no [`Ref`]: two writers +/// cannot pick the same one, and replaying the action onto another version +/// leaves it untouched. +#[derive(Debug, Clone, PartialEq)] +pub struct AddIndexSegment { + /// Identifies the segment, and names the directory its files live in. + pub uuid: Uuid, + /// The logical index this segment belongs to. + pub name: String, + /// The indexed fields. + pub fields: Vec, + /// Index-type-specific metadata, opaque to the transaction layer. + pub index_details: Option>, + pub index_version: i32, + /// The fragments this segment covers, or `None` when the coverage was not + /// recorded -- what the system indices carry. An empty list is the + /// different statement that the segment covers nothing. + pub covered_fragments: Option>, + /// The segment's files and their sizes, empty when the writer did not + /// record them. + pub files: Vec, + /// The base path the files live under, for a segment imported from another + /// dataset. `None` means the dataset's own index directory. + pub base: Option, + /// When the segment was built, or `None` when the writer did not record + /// it. Carried rather than derived, because it describes a build that + /// replaying this action does not redo. + pub created_at: Option>, + /// The dataset version whose data this segment reflects, or `None` for the + /// version the operation reads -- what a freshly built segment reflects. + /// + /// A segment merged from older ones reflects only as much as its oldest + /// input, so this is not always the read version and is not derivable from + /// it. The overlay version gate reads it: an overlay committed at or before + /// it counts as already folded into the index. + pub dataset_version: Option, + pub data_change: bool, +} + +impl AddIndexSegment { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let fields = self + .fields + .iter() + .map(|field| state.resolve_field(*field)) + .collect::>>()?; + let fragment_bitmap = self + .covered_fragments + .as_ref() + .map(|fragments| self.resolve_coverage(fragments, state)) + .transpose()?; + let base_id = self.base.map(|base| state.resolve_base(base)).transpose()?; + + state.add_index_segment(IndexMetadata { + uuid: self.uuid, + name: self.name.clone(), + fields, + dataset_version: self.reflected_version(state.read_version())?, + fragment_bitmap, + index_details: self.index_details.clone(), + index_version: self.index_version, + created_at: self.created_at, + base_id, + files: (!self.files.is_empty()).then(|| self.files.clone()), + }) + } + + /// The version this segment reflects, defaulting to the one the operation + /// reads. + fn reflected_version(&self, read_version: u64) -> Result { + let Some(version) = self.dataset_version else { + return Ok(read_version); + }; + if version > read_version { + return Err(Error::invalid_input(format!( + "AddIndexSegment for index '{}' reflects dataset version {version}, which is newer than the version {read_version} this operation reads; a segment cannot reflect data it could not have seen", + self.name + ))); + } + Ok(version) + } + + fn resolve_coverage(&self, fragments: &[Ref], state: &ApplyState) -> Result { + fragments + .iter() + .map(|fragment| { + let id = state.resolve_fragment(*fragment)?; + u32::try_from(id).map_err(|_| { + Error::invalid_input(format!( + "AddIndexSegment for index '{}' covers fragment {id}, which is beyond the \ + largest fragment id an index can record ({})", + self.name, + u32::MAX + )) + }) + }) + .collect() + } + + /// Left to the writer. An index is derived state, so building one normally + /// changes nothing a reader sees -- but the MemWAL index holds real + /// unflushed rows, so the answer is not the same for every segment. + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// The logical index, by name. Two writers may add segments to *different* + /// indices at once, and may extend one index concurrently with any change to + /// the data it covers -- an index is derived state, and a segment that has + /// fallen behind is pruned rather than being wrong. What they may not do is + /// both build the same index, which would leave two segments each claiming + /// to cover the same fragments. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::IndexName(self.name.clone())); + } +} + +impl DeepSizeOf for AddIndexSegment { + fn deep_size_of_children(&self, context: &mut Context) -> usize { + // `prost_types::Any` does not implement DeepSizeOf, but it is only a + // type url and a byte string, so both are measured directly. + let index_details = self.index_details.as_ref().map_or(0, |details| { + std::mem::size_of::() + + details.type_url.capacity() + + details.value.capacity() + }); + self.uuid.as_bytes().deep_size_of_children(context) + + self.name.deep_size_of_children(context) + + self.fields.deep_size_of_children(context) + + index_details + + self.covered_fragments.deep_size_of_children(context) + + self.files.deep_size_of_children(context) + } +} + +impl From<&AddIndexSegment> for pb::AddIndexSegment { + fn from(value: &AddIndexSegment) -> Self { + Self { + uuid: Some((&value.uuid).into()), + name: value.name.clone(), + fields: value.fields.iter().map(|field| (*field).into()).collect(), + index_details: value + .index_details + .as_ref() + .map(|details| details.as_ref().clone()), + index_version: Some(value.index_version), + covered_fragments: value.covered_fragments.as_ref().map(|fragments| { + pb::FragmentCoverage { + fragments: fragments.iter().map(|id| (*id).into()).collect(), + } + }), + files: value + .files + .iter() + .map(|file| pb::IndexFile { + path: file.path.clone(), + size_bytes: file.size_bytes, + }) + .collect(), + data_change: data_change_to_wire(value.data_change), + base: value.base.map(pb::Ref::from), + created_at: value + .created_at + .map(|created_at| created_at.timestamp_millis() as u64), + dataset_version: value.dataset_version, + } + } +} + +impl TryFrom for AddIndexSegment { + type Error = Error; + + fn try_from(message: pb::AddIndexSegment) -> Result { + let created_at = message + .created_at + .map(|millis| { + chrono::DateTime::from_timestamp_millis(millis as i64).ok_or_else(|| { + Error::invalid_input(format!( + "AddIndexSegment.created_at is {millis}ms since the epoch, which is not a \ + representable timestamp" + )) + }) + }) + .transpose()?; + + Ok(Self { + uuid: Uuid::try_from(&required(message.uuid, "AddIndexSegment.uuid")?)?, + name: message.name, + fields: message + .fields + .into_iter() + .map(Ref::try_from) + .collect::>>()?, + index_details: message.index_details.map(Arc::new), + index_version: message.index_version.unwrap_or_default(), + covered_fragments: message + .covered_fragments + .map(|coverage| { + coverage + .fragments + .into_iter() + .map(Ref::try_from) + .collect::>>() + }) + .transpose()?, + files: message + .files + .into_iter() + .map(|file| IndexFile { + path: file.path, + size_bytes: file.size_bytes, + }) + .collect(), + base: message.base.map(Ref::try_from).transpose()?, + created_at, + dataset_version: message.dataset_version, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::test_support::{ + added_field, apply_with_indices, backed_manifest, + }; + use crate::transaction::action::{Action, AddField, AddFragment, DropField}; + use crate::transaction::test_support::sample_index_metadata; + + fn segment(name: &str, fields: Vec) -> AddIndexSegment { + AddIndexSegment { + uuid: Uuid::new_v4(), + name: name.into(), + fields, + index_details: None, + index_version: 1, + covered_fragments: Some(vec![Ref::Committed(0)]), + files: Vec::new(), + base: None, + created_at: None, + dataset_version: None, + data_change: false, + } + } + + #[test] + fn test_add_index_segment_records_coverage_and_the_version_it_describes() { + let manifest = backed_manifest(); + let action = AddIndexSegment { + files: vec![IndexFile { + path: "index.idx".into(), + size_bytes: 1024, + }], + ..segment("by_a", vec![Ref::Committed(0)]) + }; + + let (next, indices) = apply_with_indices( + &manifest, + vec![Action::AddIndexSegment(action.clone())], + Vec::new(), + ) + .unwrap(); + + assert_eq!(indices.len(), 1); + let index = &indices[0]; + assert_eq!(index.uuid, action.uuid); + assert_eq!(index.name, "by_a"); + assert_eq!(index.fields, vec![0]); + assert_eq!(index.fragment_bitmap, Some([0].into_iter().collect())); + assert_eq!(index.files, Some(action.files)); + // A segment that does not say what it reflects reflects the version the + // operation read, so replaying it elsewhere restamps it. + assert_eq!(index.dataset_version, manifest.version); + assert!(index.dataset_version < next.version); + } + + #[test] + fn test_a_merged_segment_keeps_the_older_version_it_reflects() { + let manifest = backed_manifest(); + let (_, indices) = apply_with_indices( + &manifest, + vec![Action::AddIndexSegment(AddIndexSegment { + dataset_version: Some(manifest.version - 1), + ..segment("by_a", vec![Ref::Committed(0)]) + })], + Vec::new(), + ) + .unwrap(); + + assert_eq!(indices[0].dataset_version, manifest.version - 1); + } + + #[test] + fn test_a_segment_reflecting_a_future_version_is_rejected() { + let manifest = backed_manifest(); + let error = apply_with_indices( + &manifest, + vec![Action::AddIndexSegment(AddIndexSegment { + dataset_version: Some(manifest.version + 1), + ..segment("by_a", vec![Ref::Committed(0)]) + })], + Vec::new(), + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("could not have seen"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_a_second_segment_extends_the_same_index() { + let existing = sample_index_metadata("by_a"); + let added = segment("by_a", vec![Ref::Committed(0)]); + + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![Action::AddIndexSegment(added.clone())], + vec![existing.clone()], + ) + .unwrap(); + + // A logical index is the set of segments sharing a name, so adding one + // leaves the other in place rather than replacing it. + assert_eq!(indices.len(), 2); + let uuids = indices.iter().map(|index| index.uuid).collect::>(); + assert!(uuids.contains(&existing.uuid)); + assert!(uuids.contains(&added.uuid)); + } + + #[test] + fn test_re_adding_a_segment_is_rejected() { + let existing = sample_index_metadata("by_a"); + let error = apply_with_indices( + &backed_manifest(), + vec![Action::AddIndexSegment(AddIndexSegment { + uuid: existing.uuid, + ..segment("by_a", vec![Ref::Committed(0)]) + })], + vec![existing], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("added once"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_unrecorded_coverage_is_not_the_same_as_covering_nothing() { + let unknown = AddIndexSegment { + covered_fragments: None, + ..segment("system", vec![]) + }; + let empty = AddIndexSegment { + covered_fragments: Some(Vec::new()), + ..segment("empty", vec![]) + }; + + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![ + Action::AddIndexSegment(unknown), + Action::AddIndexSegment(empty), + ], + Vec::new(), + ) + .unwrap(); + + let bitmap_of = |name: &str| { + indices + .iter() + .find(|index| index.name == name) + .unwrap() + .fragment_bitmap + .clone() + }; + assert_eq!(bitmap_of("system"), None); + assert_eq!(bitmap_of("empty"), Some(RoaringBitmap::new())); + } + + #[test] + fn test_a_segment_can_cover_a_fragment_minted_in_the_same_operation() { + // Indexing what an operation just wrote is the point of composing the + // two, and the fragment has no committed id until this apply runs. + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddIndexSegment(AddIndexSegment { + covered_fragments: Some(vec![Ref::Committed(0), Ref::Local(0)]), + ..segment("by_a", vec![Ref::Committed(0)]) + }), + ], + Vec::new(), + ) + .unwrap(); + + assert_eq!( + indices[0].fragment_bitmap, + Some([0, 1].into_iter().collect()) + ); + } + + #[test] + fn test_a_segment_can_index_a_field_minted_in_the_same_operation() { + let (next, indices) = apply_with_indices( + &backed_manifest(), + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("added"), + }), + Action::AddIndexSegment(segment("by_added", vec![Ref::Local(0)])), + ], + Vec::new(), + ) + .unwrap(); + + let field = next.schema.field("added").unwrap(); + assert_eq!(indices[0].fields, vec![field.id]); + } + + #[test] + fn test_a_segment_over_a_dropped_field_does_not_survive_the_commit() { + // The assembly prunes indices whose fields left the schema, so an + // operation that drops a field and indexes it cannot smuggle one in. + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![ + Action::DropField(DropField { + field: Ref::Committed(0), + }), + Action::AddIndexSegment(segment("by_a", vec![Ref::Committed(0)])), + ], + Vec::new(), + ) + .unwrap(); + + assert!(indices.is_empty()); + } + + #[test] + fn test_two_writers_building_the_same_index_conflict() { + use crate::transaction::action::{CompositeOperation, Footprint, UserAction}; + + let footprint = |action: AddIndexSegment| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", + vec![Action::AddIndexSegment(action)], + )])) + }; + + let ours = footprint(segment("by_a", vec![Ref::Committed(0)])); + let same_name = footprint(segment("by_a", vec![Ref::Committed(0)])); + let other_name = footprint(segment("by_b", vec![Ref::Committed(1)])); + + assert!(ours.conflicts_with(&same_name)); + assert!(!ours.conflicts_with(&other_name)); + } +} diff --git a/rust/lance-table/src/transaction/action/add_overlays.rs b/rust/lance-table/src/transaction/action/add_overlays.rs new file mode 100644 index 00000000000..53b429bcd2e --- /dev/null +++ b/rust/lance-table/src/transaction/action/add_overlays.rs @@ -0,0 +1,250 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Append overlay files to a fragment. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Footprint, Ref}; +use crate::format::overlay::DataOverlayFile; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Append overlay files to one fragment, supplying new values for a subset of +/// its `(row offset, field)` cells without rewriting its base data files. +/// +/// Overlays are appended rather than replaced, so overlays a concurrent writer +/// added survive. Within the fragment they are ordered newest-last, which is +/// what appending at the version this commit produces gives. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AddOverlays { + pub fragment: Ref, + /// The overlays to append, oldest first. Each one's `committed_version` is + /// ignored and stamped with the version this commit produces, so replaying + /// the action onto a newer version re-stamps rather than backdates. + pub overlays: Vec, + pub data_change: bool, +} + +impl AddOverlays { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let fragment_id = state.resolve_fragment(self.fragment)?; + let committed_version = state.new_version(); + let fragment = state.fragment_mut(fragment_id, "AddOverlays")?; + fragment + .overlays + .extend(self.overlays.iter().cloned().map(|mut overlay| { + overlay.committed_version = committed_version; + overlay + })); + Ok(()) + } + + /// An overlay supplies new cell values, so by default it changes what a + /// reader sees. The writer can still mark a restatement of existing values + /// as no change. + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// Only that the fragment must still be there. An overlay writes no + /// coordinate of its own: two concurrent overlays over the same cells both + /// land, and the newer `committed_version` decides which value wins. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + if let Some(fragment) = self.fragment.committed() { + footprint.require_fragment(fragment); + } + } +} + +impl From<&AddOverlays> for pb::AddOverlays { + fn from(value: &AddOverlays) -> Self { + Self { + fragment: Some(value.fragment.into()), + overlays: value + .overlays + .iter() + .map(pb::DataOverlayFile::from) + .collect(), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for AddOverlays { + type Error = Error; + + fn try_from(message: pb::AddOverlays) -> Result { + Ok(Self { + fragment: required(message.fragment, "AddOverlays.fragment")?.try_into()?, + overlays: message + .overlays + .into_iter() + .map(DataOverlayFile::try_from) + .collect::>>()?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::format::overlay::OverlayCoverage; + use crate::transaction::action::test_support::{apply, backed_manifest}; + use crate::transaction::action::{ + Action, AddFragment, CompositeOperation, RemoveFragment, UserAction, + }; + use lance_file::version::ConcreteFileVersion; + use roaring::RoaringBitmap; + use std::sync::Arc; + + fn overlay(path: &str, offsets: &[u32]) -> DataOverlayFile { + DataOverlayFile { + data_file: DataFile::new( + path, + vec![0], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + ), + coverage: OverlayCoverage::Shared(Arc::new( + offsets.iter().copied().collect::(), + )), + // Whatever the writer left here is overwritten at apply. + committed_version: 0, + } + } + + fn add(fragment: Ref, overlays: Vec) -> Action { + Action::AddOverlays(AddOverlays { + fragment, + overlays, + data_change: true, + }) + } + + fn footprint(actions: Vec) -> Footprint { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + } + + #[test] + fn test_overlays_are_stamped_with_the_version_the_commit_produces() { + let manifest = backed_manifest(); + let expected = manifest.version + 1; + + let out = apply( + &manifest, + vec![add(Ref::Committed(0), vec![overlay("a.lance", &[1, 2])])], + ) + .unwrap(); + + let overlays = &out.fragments[0].overlays; + assert_eq!(overlays.len(), 1); + assert_eq!(overlays[0].committed_version, expected); + } + + #[test] + fn test_overlays_are_appended_to_the_ones_already_there() { + let mut manifest = backed_manifest(); + let mut fragment = manifest.fragments[0].clone(); + fragment.overlays.push(overlay("old.lance", &[0])); + manifest.fragments = Arc::new(vec![fragment]); + + let out = apply( + &manifest, + vec![add(Ref::Committed(0), vec![overlay("new.lance", &[1])])], + ) + .unwrap(); + + let paths = out.fragments[0] + .overlays + .iter() + .map(|overlay| overlay.data_file.path.as_str()) + .collect::>(); + assert_eq!(paths, vec!["old.lance", "new.lance"]); + } + + #[test] + fn test_several_overlays_keep_the_order_they_were_given_in() { + let out = apply( + &backed_manifest(), + vec![add( + Ref::Committed(0), + vec![overlay("first.lance", &[0]), overlay("second.lance", &[0])], + )], + ) + .unwrap(); + + let paths = out.fragments[0] + .overlays + .iter() + .map(|overlay| overlay.data_file.path.as_str()) + .collect::>(); + assert_eq!(paths, vec!["first.lance", "second.lance"]); + } + + #[test] + fn test_an_overlay_can_target_a_fragment_this_operation_minted() { + let out = apply( + &backed_manifest(), + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + add(Ref::Local(0), vec![overlay("a.lance", &[0])]), + ], + ) + .unwrap(); + + let minted = out.fragments.iter().find(|f| f.id == 1).unwrap(); + assert_eq!(minted.overlays.len(), 1); + } + + #[test] + fn test_overlaying_a_fragment_that_is_not_there_is_rejected() { + let error = apply( + &backed_manifest(), + vec![add(Ref::Committed(42), vec![overlay("a.lance", &[0])])], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error + .to_string() + .contains("AddOverlays targets fragment 42, which does not exist"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_two_writers_overlaying_the_same_fragment_do_not_conflict() { + let ours = footprint(vec![add(Ref::Committed(0), vec![overlay("a.lance", &[0])])]); + let theirs = footprint(vec![add(Ref::Committed(0), vec![overlay("b.lance", &[0])])]); + + assert!(!ours.conflicts_with(&theirs)); + } + + #[test] + fn test_overlaying_a_fragment_a_concurrent_writer_removes_conflicts() { + let ours = footprint(vec![add(Ref::Committed(0), vec![overlay("a.lance", &[0])])]); + let theirs = footprint(vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(0), + data_change: true, + })]); + + assert!(ours.conflicts_with(&theirs)); + assert!(theirs.conflicts_with(&ours)); + } +} diff --git a/rust/lance-table/src/transaction/action/adjust_index_coverage.rs b/rust/lance-table/src/transaction/action/adjust_index_coverage.rs new file mode 100644 index 00000000000..ef41f6e5927 --- /dev/null +++ b/rust/lance-table/src/transaction/action/adjust_index_coverage.rs @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Adjust the fragment coverage of an index segment. + +use super::apply::ApplyState; +use super::proto::required; +use super::{Coordinate, Footprint, Ref}; +use crate::format::pb; +use lance_core::deepsize::{Context, DeepSizeOf}; +use lance_core::{Error, Result}; +use uuid::Uuid; + +/// Adjust which fragments an existing index segment covers, without rewriting +/// the segment. +/// +/// This is how a segment picks up fragments a compaction produced, or lets go +/// of ones it no longer describes, in cases where the index files themselves are +/// still good. Rewriting a segment's contents is a +/// [`RemoveIndexSegment`](super::RemoveIndexSegment) plus an +/// [`AddIndexSegment`](super::AddIndexSegment) instead. +/// +/// Additions are applied before removals, so a fragment named on both sides ends +/// up outside the coverage. +/// +/// # When a fragment may be added +/// +/// Coverage says which fragments a segment's existing index files describe, so a +/// fragment may only be added when they already describe its rows. In practice +/// that means one case: a rewrite -- compaction, or an in-place column rewrite -- +/// moved rows the segment already covered into a new fragment, and the segment +/// reaches them through the fragment-reuse remapping. The matching removal of the +/// old fragment ids belongs in the same action. +/// +/// Adding a fragment of genuinely new rows is a writer error, even though nothing +/// here can detect it: the segment has no entries for those rows, so a query +/// served by the index would silently miss them. New rows are covered by building +/// a segment over them ([`AddIndexSegment`](super::AddIndexSegment)), not by +/// widening an existing one. +#[derive(Debug, Clone, PartialEq)] +pub struct AdjustIndexCoverage { + pub uuid: Uuid, + /// Fragments to bring into the coverage. A [`Ref::Local`] names one this + /// operation minted, which is how a segment follows its rows into the + /// fragment a rewrite in the same operation moved them to. + pub add_fragments: Vec, + /// Committed fragment ids to drop from the coverage. Unlike the additions + /// these take no [`Ref`]: a fragment minted in this same operation was not + /// in the coverage to begin with. + pub remove_fragments: Vec, +} + +impl AdjustIndexCoverage { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let added = self + .add_fragments + .iter() + .map(|fragment| self.coverage_id(state.resolve_fragment(*fragment)?)) + .collect::>>()?; + let removed = self + .remove_fragments + .iter() + .map(|fragment| self.coverage_id(*fragment)) + .collect::>>()?; + + let segment = state.index_segment_mut(self.uuid)?; + let Some(bitmap) = segment.fragment_bitmap.as_mut() else { + return Err(Error::invalid_input(format!( + "index segment {} records no fragment coverage, so there is nothing to adjust; \ + rewrite the segment to give it coverage", + self.uuid + ))); + }; + for fragment in added { + bitmap.insert(fragment); + } + for fragment in removed { + bitmap.remove(fragment); + } + Ok(()) + } + + fn coverage_id(&self, fragment: u64) -> Result { + u32::try_from(fragment).map_err(|_| { + Error::invalid_input(format!( + "AdjustIndexCoverage for segment {} names fragment {fragment}, which is beyond \ + the largest fragment id an index can record ({})", + self.uuid, + u32::MAX + )) + }) + } + + /// Coverage says which fragments an index describes, never what any of them + /// hold, so adjusting it cannot change a row a reader sees. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// The segment, by uuid, as for a removal: this rewrites part of one + /// segment and nothing else. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::IndexSegment(self.uuid)); + } +} + +impl DeepSizeOf for AdjustIndexCoverage { + fn deep_size_of_children(&self, context: &mut Context) -> usize { + self.uuid.as_bytes().deep_size_of_children(context) + + self.add_fragments.deep_size_of_children(context) + + self.remove_fragments.deep_size_of_children(context) + } +} + +impl From<&AdjustIndexCoverage> for pb::AdjustIndexCoverage { + fn from(value: &AdjustIndexCoverage) -> Self { + Self { + uuid: Some((&value.uuid).into()), + add_fragments: value + .add_fragments + .iter() + .map(|fragment| (*fragment).into()) + .collect(), + remove_fragments: value.remove_fragments.clone(), + } + } +} + +impl TryFrom for AdjustIndexCoverage { + type Error = Error; + + fn try_from(message: pb::AdjustIndexCoverage) -> Result { + Ok(Self { + uuid: Uuid::try_from(&required(message.uuid, "AdjustIndexCoverage.uuid")?)?, + add_fragments: message + .add_fragments + .into_iter() + .map(Ref::try_from) + .collect::>>()?, + remove_fragments: message.remove_fragments, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::IndexMetadata; + use crate::transaction::action::test_support::{apply_with_indices, backed_manifest}; + use crate::transaction::action::{ + Action, AddFragment, CompositeOperation, Footprint, UserAction, + }; + use crate::transaction::test_support::sample_index_metadata; + + fn covering(name: &str, fragments: impl IntoIterator) -> IndexMetadata { + IndexMetadata { + fragment_bitmap: Some(fragments.into_iter().collect()), + ..sample_index_metadata(name) + } + } + + fn adjust(uuid: Uuid, add: Vec, remove: Vec) -> Action { + Action::AdjustIndexCoverage(AdjustIndexCoverage { + uuid, + add_fragments: add, + remove_fragments: remove, + }) + } + + #[test] + fn test_adjust_index_coverage_adds_and_removes() { + let segment = covering("by_a", [0, 1]); + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![adjust(segment.uuid, vec![Ref::Committed(2)], vec![1])], + vec![segment], + ) + .unwrap(); + + assert_eq!( + indices[0].fragment_bitmap, + Some([0, 2].into_iter().collect()) + ); + } + + #[test] + fn test_coverage_can_take_in_a_fragment_minted_in_the_same_operation() { + // Appending and extending an index's reach over what was appended is + // one operation, and the fragment has no committed id until apply. + let segment = covering("by_a", [0]); + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + adjust(segment.uuid, vec![Ref::Local(0)], vec![]), + ], + vec![segment], + ) + .unwrap(); + + assert_eq!( + indices[0].fragment_bitmap, + Some([0, 1].into_iter().collect()) + ); + } + + #[test] + fn test_a_fragment_named_on_both_sides_ends_up_uncovered() { + let segment = covering("by_a", [0]); + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![adjust(segment.uuid, vec![Ref::Committed(5)], vec![5])], + vec![segment], + ) + .unwrap(); + + assert_eq!(indices[0].fragment_bitmap, Some([0].into_iter().collect())); + } + + #[test] + fn test_adjusting_a_segment_that_is_not_there_is_rejected() { + let error = apply_with_indices( + &backed_manifest(), + vec![adjust(Uuid::from_u128(1), vec![Ref::Committed(0)], vec![])], + vec![covering("by_a", [0])], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("is not part of the dataset"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_adjusting_a_segment_with_no_recorded_coverage_is_rejected() { + // Coverage of "unknown" is not an empty set to add to: turning it into + // a concrete set would narrow what the segment serves, silently. + let segment = IndexMetadata { + fragment_bitmap: None, + ..sample_index_metadata("system") + }; + let error = apply_with_indices( + &backed_manifest(), + vec![adjust(segment.uuid, vec![Ref::Committed(0)], vec![])], + vec![segment], + ) + .unwrap_err(); + + assert!( + error.to_string().contains("records no fragment coverage"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_two_writers_adjusting_the_same_segment_conflict() { + let footprint = |actions| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + }; + + let uuid = Uuid::from_u128(1); + let ours = footprint(vec![adjust(uuid, vec![Ref::Committed(1)], vec![])]); + let same = footprint(vec![adjust(uuid, vec![], vec![2])]); + let other = footprint(vec![adjust(Uuid::from_u128(2), vec![], vec![2])]); + + assert!(ours.conflicts_with(&same)); + assert!(!ours.conflicts_with(&other)); + } +} diff --git a/rust/lance-table/src/transaction/action/alter_field.rs b/rust/lance-table/src/transaction/action/alter_field.rs new file mode 100644 index 00000000000..4c475ae7735 --- /dev/null +++ b/rust/lance-table/src/transaction/action/alter_field.rs @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Alter facets of an existing field in place. + +use super::apply::ApplyState; +use super::proto::required; +use super::{Footprint, Ref}; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Alter facets of an existing field in place, preserving its id. +/// +/// Each facet is independently optional -- present means "change this", absent +/// means "leave it alone" -- so a widening cast and a nullability relaxation on +/// the same field commute. A cast additionally needs a +/// [`TombstoneFieldData`](super::TombstoneFieldData) plus a fresh +/// [`AddDataFile`](super::AddDataFile) to rewrite the data. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AlterField { + pub field: Ref, + pub name: Option, + /// The new Arrow logical type. The cast. + pub logical_type: Option, + pub nullable: Option, +} + +impl AlterField { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let field_id = state.resolve_field(self.field)?; + let field = state + .schema_mut() + .field_by_id_mut(field_id) + .ok_or_else(|| { + Error::invalid_input(format!( + "AlterField names field {field_id}, which does not exist" + )) + })?; + if let Some(name) = &self.name { + field.name.clone_from(name); + } + if let Some(nullable) = self.nullable { + field.nullable = nullable; + } + if let Some(logical_type) = &self.logical_type { + field.logical_type = logical_type.as_str().into(); + // The cast leaves any index on the field describing the old type. + // The data rewrite itself is separate actions; this only records + // that every fragment's view of the field changed. + state.rebind_field_everywhere(field_id); + } + Ok(()) + } + + /// Renaming or relaxing a field leaves the values alone, and the + /// rewrite a cast needs is separate actions that answer for themselves. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// The field's definition. The data rewrite a cast needs is separate + /// actions, which record their own coordinates. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add_field_definition(self.field); + } +} + +impl From<&AlterField> for pb::AlterField { + fn from(value: &AlterField) -> Self { + Self { + field: Some(value.field.into()), + name: value.name.clone(), + logical_type: value.logical_type.clone(), + nullable: value.nullable, + } + } +} + +impl TryFrom for AlterField { + type Error = Error; + + fn try_from(message: pb::AlterField) -> Result { + Ok(Self { + field: required(message.field, "AlterField.field")?.try_into()?, + name: message.name, + logical_type: message.logical_type, + nullable: message.nullable, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::Action; + use crate::transaction::action::test_support::{apply, apply_with_indices, backed_manifest}; + use crate::transaction::test_support::sample_index_metadata; + + #[test] + fn test_alter_field_renames_without_touching_indices() { + let (next, indices) = apply_with_indices( + &backed_manifest(), + vec![Action::AlterField(AlterField { + field: Ref::Committed(0), + name: Some("renamed".into()), + logical_type: None, + nullable: Some(true), + })], + vec![sample_index_metadata("idx")], + ) + .unwrap(); + + let field = next.schema.field_by_id(0).unwrap(); + assert_eq!(field.name, "renamed"); + assert!(field.nullable); + // A rename does not change the values the index recorded. + assert!(indices[0].fragment_bitmap.as_ref().unwrap().contains(0)); + } + + #[test] + fn test_alter_field_retype_prunes_covering_indices() { + let (next, indices) = apply_with_indices( + &backed_manifest(), + vec![Action::AlterField(AlterField { + field: Ref::Committed(0), + name: None, + logical_type: Some("int64".into()), + nullable: None, + })], + vec![sample_index_metadata("idx")], + ) + .unwrap(); + + assert_eq!( + next.schema.field_by_id(0).unwrap().logical_type.to_string(), + "int64" + ); + assert!(indices[0].fragment_bitmap.as_ref().unwrap().is_empty()); + } + + #[test] + fn test_alter_field_rejects_a_missing_field() { + let error = apply( + &backed_manifest(), + vec![Action::AlterField(AlterField { + field: Ref::Committed(7), + name: Some("nope".into()), + logical_type: None, + nullable: None, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("field 7"), "{error}"); + } + + #[test] + fn test_alter_field_can_name_a_field_minted_in_the_same_operation() { + // A squash of "add column" and "rename column" lowers to exactly this, + // and has no committed id to name the field by. + use crate::transaction::action::AddField; + use crate::transaction::action::test_support::added_field; + + let next = apply( + &backed_manifest(), + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("before"), + }), + Action::AlterField(AlterField { + field: Ref::Local(0), + name: Some("after".into()), + logical_type: None, + nullable: None, + }), + ], + ) + .unwrap(); + + assert!(next.schema.field("before").is_none()); + assert!(next.schema.field("after").is_some()); + } +} diff --git a/rust/lance-table/src/transaction/action/apply.rs b/rust/lance-table/src/transaction/action/apply.rs new file mode 100644 index 00000000000..488147dfa30 --- /dev/null +++ b/rust/lance-table/src/transaction/action/apply.rs @@ -0,0 +1,618 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Applying an action set to produce the next manifest. +//! +//! Each action is applied in order against a working copy of the read-version +//! state. Minting actions allocate an id from the target's counters as they are +//! reached and record it against their local token, so every later reference to +//! that token -- in this same operation -- resolves to the id this apply chose. +//! Replaying the same action set against a different version therefore produces +//! different ids without any of the actions changing. +//! +//! [`ApplyState`] is that working copy, and its methods are the API the action +//! modules program against. What each action does with it lives in that action's +//! own module. + +use super::{CompositeOperation, Ref}; +use crate::format::{BasePath, Fragment, IndexMetadata, Manifest, ManifestBuildConfig}; +use crate::rowids::version::build_version_meta; +use crate::system_index::mem_wal::{CompactedSsTable, update_mem_wal_index_compacted_sstables}; +use crate::transaction::Transaction; +use lance_core::datatypes::Schema; +use lance_core::{Error, Result}; +use std::collections::{HashMap, HashSet}; +use uuid::Uuid; + +/// The field id written into a data file's field list once the file no longer +/// backs that field. A file whose every slot is tombstoned is dropped. +pub(super) const TOMBSTONED_FIELD: i32 = -2; + +impl Transaction { + /// Build the next manifest by applying an action set. + /// + /// Unlike the legacy path this always requires a current manifest: an action + /// set describes a delta, so there is nothing for it to be a delta against + /// when the dataset does not exist yet. + pub(in crate::transaction) fn build_manifest_from_actions( + &self, + composite_operation: &CompositeOperation, + current_manifest: Option<&Manifest>, + current_indices: Vec, + transaction_file_path: &str, + config: &ManifestBuildConfig, + ) -> Result<(Manifest, Vec)> { + let current_manifest = current_manifest.ok_or_else(|| { + Error::invalid_input( + "an action-based transaction describes a change to an existing dataset; \ + it cannot create one", + ) + })?; + if config.use_stable_row_ids && !current_manifest.uses_stable_row_ids() { + return Err(Error::not_supported_source( + "Cannot enable stable row ids on existing dataset".into(), + )); + } + + let mut state = ApplyState::new(current_manifest, current_indices, config); + for action in composite_operation.iter_actions() { + action.apply(&mut state)?; + } + + state.into_manifest(self, transaction_file_path) + } +} + +/// The read-version state an action set is applied against, plus the id +/// allocations made so far. +pub(super) struct ApplyState<'a> { + /// The read version this delta applies to. + current_manifest: &'a Manifest, + /// How the manifest this delta produces is to be assembled. + build_config: &'a ManifestBuildConfig, + schema: Schema, + fragments: Vec, + /// Base paths minted by this operation. Kept apart from the manifest's own + /// base paths, which the manifest assembly inherits from the read version. + new_bases: Vec, + existing_base_paths: HashMap, + /// The index segments, as the actions have left them so far. Index + /// actions edit this list; the assembly then prunes whatever the data + /// actions invalidated. + indices: Vec, + /// The manifest's string maps. Unlike the schema and the fragment list, + /// these are inherited wholesale by the manifest assembly, so an edit has + /// to be written back over the assembled manifest. + config: HashMap, + table_metadata: HashMap, + + next_fragment_id: u64, + next_field_id: i32, + next_base_id: u32, + + /// Local token -> the id minted for it, one map per id space. + fragment_tokens: HashMap, + field_tokens: HashMap, + base_tokens: HashMap, + + /// Ids of the fragments this operation minted. + minted_fragments: HashSet, + + /// The highest fragment id this operation reserved for a later writer, if + /// it reserved any. No fragment backs it, so the manifest assembly cannot + /// infer it from the fragment list. + reserved_fragment_ids: Option, + + /// Fields whose backing data changed, per fragment. An index covering such + /// a field no longer describes that fragment's contents. + rebound_fields: HashMap>, +} + +impl<'a> ApplyState<'a> { + fn new( + manifest: &'a Manifest, + indices: Vec, + build_config: &'a ManifestBuildConfig, + ) -> Self { + Self { + current_manifest: manifest, + build_config, + schema: manifest.schema.clone(), + fragments: manifest.fragments.as_ref().clone(), + indices, + new_bases: Vec::new(), + existing_base_paths: manifest.base_paths.clone(), + config: manifest.config.clone(), + table_metadata: manifest.table_metadata.clone(), + next_fragment_id: manifest.max_fragment_id().map(|id| id + 1).unwrap_or(0), + next_field_id: manifest.max_field_id() + 1, + next_base_id: manifest + .base_paths + .keys() + .max() + .map(|id| id + 1) + .unwrap_or(1), + fragment_tokens: HashMap::new(), + field_tokens: HashMap::new(), + base_tokens: HashMap::new(), + minted_fragments: HashSet::new(), + reserved_fragment_ids: None, + rebound_fields: HashMap::new(), + } + } + + /// Assemble the next manifest from the state the actions left behind. + fn into_manifest( + mut self, + transaction: &Transaction, + transaction_file_path: &str, + ) -> Result<(Manifest, Vec)> { + let current_manifest = self.current_manifest; + let config = self.build_config; + let new_version = current_manifest.version + 1; + + let mut next_row_id = current_manifest + .uses_stable_row_ids() + .then_some(current_manifest.next_row_id); + self.assign_row_ids_to_minted_fragments(&mut next_row_id, new_version)?; + + let ApplyState { + schema, + mut fragments, + mut indices, + new_bases, + rebound_fields, + reserved_fragment_ids, + config: dataset_config, + table_metadata, + .. + } = self; + + prune_rebound_fields_from_indices(&mut indices, &rebound_fields); + Transaction::retain_relevant_indices(&mut indices, &schema, &fragments); + + Transaction::normalize_fragments(&mut fragments)?; + let mut manifest = transaction.assemble_manifest( + Some(current_manifest), + schema, + fragments, + HashMap::new(), + false, + config, + )?; + + for base in new_bases { + manifest.base_paths.insert(base.id, base); + } + + manifest.config = dataset_config; + manifest.table_metadata = table_metadata; + + // A reserved id backs no fragment, so the manifest assembly cannot + // derive it from the fragment list; raise the high-water mark to cover + // the range so a later writer's ids are not handed out twice. + if let Some(high_water) = reserved_fragment_ids { + let high_water = u32::try_from(high_water).map_err(|_| { + Error::invalid_input(format!( + "reserving fragment ids up to {high_water} exceeds the maximum fragment id \ + ({})", + u32::MAX + )) + })?; + manifest.max_fragment_id = Some( + manifest + .max_fragment_id + .map_or(high_water, |current| current.max(high_water)), + ); + } + + manifest.transaction_file = Some(transaction_file_path.to_string()); + if let Some(next_row_id) = next_row_id { + manifest.next_row_id = next_row_id; + } + + Ok((manifest, indices)) + } + + /// The version this delta applies to, which is the newest data an index + /// segment added by this operation can have been built from. + pub(super) fn read_version(&self) -> u64 { + self.current_manifest.version + } + + /// The version this delta produces, for the actions that stamp it into what + /// they write. Distinct from [`Self::read_version`]: a retry against a newer + /// manifest re-runs the apply, so anything stamped with this is re-stamped + /// rather than carried over. + pub(super) fn new_version(&self) -> u64 { + self.current_manifest.version + 1 + } + + /// Add an index segment. A segment's uuid identifies it, so re-adding one + /// that is already there is a mistake rather than a replacement -- swapping + /// a segment out is a [`RemoveIndexSegment`](super::RemoveIndexSegment) + /// followed by an add. + pub(super) fn add_index_segment(&mut self, segment: IndexMetadata) -> Result<()> { + if self.indices.iter().any(|index| index.uuid == segment.uuid) { + return Err(Error::invalid_input(format!( + "index segment {} is already part of the dataset; a segment is added once", + segment.uuid + ))); + } + self.indices.push(segment); + Ok(()) + } + + /// Drop an index segment by uuid. A removal that names a segment the + /// dataset does not have is a mistake rather than a no-op: it means the + /// operation was planned against a different set of segments. + pub(super) fn remove_index_segment(&mut self, uuid: Uuid) -> Result<()> { + let before = self.indices.len(); + self.indices.retain(|index| index.uuid != uuid); + if self.indices.len() == before { + return Err(Error::invalid_input(format!( + "index segment {uuid} is not part of the dataset, so it cannot be removed" + ))); + } + Ok(()) + } + + /// Record MemWAL SSTable compaction progress, creating the MemWAL index + /// when the dataset does not have one yet. + pub(super) fn update_compacted_sstables( + &mut self, + compacted_sstables: Vec, + ) -> Result<()> { + let new_version = self.new_version(); + update_mem_wal_index_compacted_sstables(&mut self.indices, new_version, compacted_sstables) + } + + pub(super) fn index_segment_mut(&mut self, uuid: Uuid) -> Result<&mut IndexMetadata> { + self.indices + .iter_mut() + .find(|index| index.uuid == uuid) + .ok_or_else(|| { + Error::invalid_input(format!( + "index segment {uuid} is not part of the dataset, so it cannot be adjusted" + )) + }) + } + + /// Whether the dataset carries stable row ids, and with them the per-row + /// version sequences. + pub(super) fn uses_stable_row_ids(&self) -> bool { + self.current_manifest.uses_stable_row_ids() + } + + pub(super) fn schema(&self) -> &Schema { + &self.schema + } + + pub(super) fn schema_mut(&mut self) -> &mut Schema { + &mut self.schema + } + + pub(super) fn config_mut(&mut self) -> &mut HashMap { + &mut self.config + } + + pub(super) fn table_metadata_mut(&mut self) -> &mut HashMap { + &mut self.table_metadata + } + + pub(super) fn fragments_mut(&mut self) -> &mut [Fragment] { + &mut self.fragments + } + + pub(super) fn fragment_mut(&mut self, fragment_id: u64, action: &str) -> Result<&mut Fragment> { + self.fragments + .iter_mut() + .find(|fragment| fragment.id == fragment_id) + .ok_or_else(|| { + Error::invalid_input(format!( + "{action} targets fragment {fragment_id}, which does not exist" + )) + }) + } + + pub(super) fn push_fragment(&mut self, fragment: Fragment) { + self.fragments.push(fragment); + } + + /// Drop a fragment and forget everything recorded about it. `false` if no + /// such fragment was present. + pub(super) fn remove_fragment(&mut self, fragment_id: u64) -> bool { + let before = self.fragments.len(); + self.fragments.retain(|fragment| fragment.id != fragment_id); + if self.fragments.len() == before { + return false; + } + self.minted_fragments.remove(&fragment_id); + self.rebound_fields.remove(&fragment_id); + true + } + + /// Empty the table: no schema, no fragments, no indices. The id counters + /// keep going, so a field or fragment added afterwards never reuses an id an + /// old file might still name. + pub(super) fn reset(&mut self) { + self.schema.fields.clear(); + self.schema.metadata.clear(); + self.fragments.clear(); + self.indices.clear(); + self.minted_fragments.clear(); + self.rebound_fields.clear(); + } + + /// The base paths this apply can see: the read version's, plus the ones + /// earlier actions in this operation minted. + pub(super) fn bases(&self) -> impl Iterator { + self.existing_base_paths + .values() + .chain(self.new_bases.iter()) + } + + pub(super) fn push_base(&mut self, base: BasePath) { + self.new_bases.push(base); + } + + pub(super) fn mint_fragment(&mut self, token: u32) -> Result { + if self.fragment_tokens.contains_key(&token) { + return Err(duplicate_token_err("fragment", token)); + } + let id = self.next_fragment_id; + self.next_fragment_id += 1; + self.fragment_tokens.insert(token, id); + self.minted_fragments.insert(id); + Ok(id) + } + + /// Take `count` ids off the fragment counter without minting fragments for + /// them. The reserved ids are `[next, next + count)`; a later writer names + /// them as [`Ref::Committed`]. + pub(super) fn reserve_fragment_ids(&mut self, count: u32) { + if count == 0 { + return; + } + self.next_fragment_id += u64::from(count); + self.reserved_fragment_ids = Some(self.next_fragment_id - 1); + } + + pub(super) fn mint_field(&mut self, token: u32) -> Result { + if self.field_tokens.contains_key(&token) { + return Err(duplicate_token_err("field", token)); + } + let id = self.next_field_id; + self.next_field_id += 1; + self.field_tokens.insert(token, id); + Ok(id) + } + + pub(super) fn mint_base(&mut self, token: u32) -> Result { + if self.base_tokens.contains_key(&token) { + return Err(duplicate_token_err("base", token)); + } + let id = self.next_base_id; + self.next_base_id += 1; + self.base_tokens.insert(token, id); + Ok(id) + } + + pub(super) fn resolve_fragment(&self, reference: Ref) -> Result { + match reference { + Ref::Committed(id) => Ok(id), + Ref::Local(token) => self + .fragment_tokens + .get(&token) + .copied() + .ok_or_else(|| unbound_token_err("fragment", token)), + } + } + + pub(super) fn resolve_base(&self, reference: Ref) -> Result { + match reference { + Ref::Committed(id) => u32::try_from(id).map_err(|_| { + Error::invalid_input(format!("base id {id} in an action is out of range")) + }), + Ref::Local(token) => self + .base_tokens + .get(&token) + .copied() + .ok_or_else(|| unbound_token_err("base", token)), + } + } + + pub(super) fn resolve_field(&self, reference: Ref) -> Result { + match reference { + Ref::Committed(id) => i32::try_from(id).map_err(|_| { + Error::invalid_input(format!("field id {id} in an action is out of range")) + }), + Ref::Local(token) => self + .field_tokens + .get(&token) + .copied() + .ok_or_else(|| unbound_token_err("field", token)), + } + } + + /// Record that these fields' data in this fragment no longer matches what + /// an index built over them recorded. + pub(super) fn rebind_fields( + &mut self, + fragment_id: u64, + fields: impl IntoIterator, + ) { + self.rebound_fields + .entry(fragment_id) + .or_default() + .extend(fields); + } + + /// As [`Self::rebind_fields`], for a change that invalidates the field + /// across every fragment at once. + pub(super) fn rebind_field_everywhere(&mut self, field: i32) { + let fragment_ids = self + .fragments + .iter() + .map(|fragment| fragment.id) + .collect::>(); + for fragment_id in fragment_ids { + self.rebound_fields + .entry(fragment_id) + .or_default() + .insert(field); + } + } + + /// Stamp row ids and version metadata onto the fragments this operation + /// minted, matching what an Append does for its new fragments. + fn assign_row_ids_to_minted_fragments( + &mut self, + next_row_id: &mut Option, + new_version: u64, + ) -> Result<()> { + let Some(next_row_id) = next_row_id.as_mut() else { + return Ok(()); + }; + let minted_ids = std::mem::take(&mut self.minted_fragments); + // The manifest assembly sorts fragments by id, so partitioning them here + // does not disturb the final order. + let (mut minted, existing): (Vec, Vec) = self + .fragments + .drain(..) + .partition(|fragment| minted_ids.contains(&fragment.id)); + + Transaction::assign_row_ids(next_row_id, minted.as_mut_slice())?; + for fragment in minted.iter_mut() { + // An action may carry its own sequences (a squashed operation + // does); only stamp the ones it left for apply to fill. + let version_meta = build_version_meta(fragment, new_version); + if fragment.last_updated_at_version_meta.is_none() { + fragment.last_updated_at_version_meta = version_meta.clone(); + } + if fragment.created_at_version_meta.is_none() { + fragment.created_at_version_meta = version_meta; + } + } + + self.fragments = existing; + self.fragments.extend(minted); + self.minted_fragments = minted_ids; + Ok(()) + } +} + +/// Drop the fragments whose data no longer matches what an index recorded. +/// +/// An index built over a field describes the values that were in that field +/// when it was built. Rebinding the field's data in a fragment invalidates the +/// index for that fragment only, so the fragment leaves the bitmap rather than +/// the whole index being discarded. +fn prune_rebound_fields_from_indices( + indices: &mut [IndexMetadata], + rebound: &HashMap>, +) { + if rebound.is_empty() { + return; + } + for index in indices.iter_mut() { + let Some(bitmap) = index.fragment_bitmap.as_mut() else { + continue; + }; + for (fragment_id, fields) in rebound { + if index.fields.iter().any(|field| fields.contains(field)) + && let Ok(fragment_id) = u32::try_from(*fragment_id) + { + bitmap.remove(fragment_id); + } + } + } +} + +fn unbound_token_err(space: &str, token: u32) -> Error { + Error::invalid_input(format!( + "an action references local {space} token {token}, which no earlier action in this \ + operation minted" + )) +} + +fn duplicate_token_err(space: &str, token: u32) -> Error { + Error::invalid_input(format!( + "local {space} token {token} is minted more than once in this operation; tokens must be \ + distinct within an operation" + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::Operation; + use crate::transaction::action::test_support::{added_field, apply, backed_manifest}; + use crate::transaction::action::{Action, AddDataFile, AddField, AddFragment}; + use crate::transaction::test_support::default_build_config; + use lance_file::version::ConcreteFileVersion; + + #[test] + fn test_an_action_set_relocates_onto_a_newer_version() { + let actions = vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("added"), + }), + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: DataFile::new_unstarted("data/new.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Local(0)], + data_change: true, + }), + ]; + + // Replaying the very same actions against the manifest the first run + // produced re-resolves both local tokens against the newer counters. + let first = apply(&backed_manifest(), actions.clone()).unwrap(); + let second = apply(&first, actions).unwrap(); + + assert_eq!( + second.fragments.iter().map(|f| f.id).collect::>(), + vec![0, 1, 2] + ); + assert_eq!( + second + .schema + .fields_pre_order() + .map(|field| field.id) + .collect::>(), + vec![0, 1, 2] + ); + // The second run's data file points at the field the second run minted, + // not at the one the first run did. + let relocated = second.fragments.iter().find(|f| f.id == 2).unwrap(); + assert_eq!(relocated.files[0].fields.as_ref(), &[2]); + } + + #[test] + fn test_action_set_cannot_create_a_dataset() { + let transaction = Transaction::new( + 0, + Operation::CompositeOperation(CompositeOperation::new(vec![])), + None, + ); + let error = transaction + .build_manifest(None, Vec::new(), "tx.txn", &default_build_config()) + .unwrap_err(); + assert!( + error.to_string().contains("cannot create one"), + "unexpected error: {error}" + ); + } +} diff --git a/rust/lance-table/src/transaction/action/assert_unique_keys.rs b/rust/lance-table/src/transaction/action/assert_unique_keys.rs new file mode 100644 index 00000000000..b948334edfd --- /dev/null +++ b/rust/lance-table/src/transaction/action/assert_unique_keys.rs @@ -0,0 +1,240 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Assert that no concurrent commit inserted a colliding key. + +use super::apply::ApplyState; +use super::proto::required; +use super::{Footprint, Ref}; +use crate::format::key_existence::KeyExistenceFilter; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// A precondition rather than a delta: the keys this operation inserts must not +/// collide with keys a concurrent commit inserted. +/// +/// This is the home for merge-insert's strict primary-key conflict detection. +/// The key columns are an unenforced primary key, so nothing in the manifest +/// records which keys exist -- the filter of inserted key hashes travels with +/// the operation because it cannot be recovered from any post-image. +/// +/// Two operations that both carry one are compatible when they agree on the key +/// columns and their filters do not intersect. An operation carrying one is not +/// compatible with a concurrent operation that inserts rows without saying which +/// keys they carry, because there is nothing to compare against. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct AssertUniqueKeys { + /// The key columns, in order. This is the authoritative list; the field ids + /// the filter carries are an artifact of the shared filter type and are left + /// empty here. + pub key_fields: Vec, + pub filter: KeyExistenceFilter, +} + +impl AssertUniqueKeys { + /// Nothing to apply -- the assertion is checked when two operations are + /// compared, not when one is folded into a manifest. The key columns are + /// still resolved and looked up, so an assertion naming a field that is not + /// there fails at the commit that carries it rather than silently guarding + /// nothing. + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + if self.key_fields.is_empty() { + return Err(Error::invalid_input( + "AssertUniqueKeys names no key column, so there is nothing for it to assert", + )); + } + for key_field in &self.key_fields { + let field_id = state.resolve_field(*key_field)?; + if state.schema().field_by_id(field_id).is_none() { + return Err(Error::invalid_input(format!( + "AssertUniqueKeys names key field {field_id}, which is not in the schema" + ))); + } + } + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + false + } + + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.assert_unique_keys(self.key_fields.clone(), self.filter.clone()); + } +} + +impl From<&AssertUniqueKeys> for pb::AssertUniqueKeys { + fn from(value: &AssertUniqueKeys) -> Self { + let mut filter = pb::KeyExistenceFilter::from(&value.filter); + // `key_fields` is the authoritative list; the filter's own copy is an + // artifact of the type it shares with the legacy Update operation. + filter.field_ids.clear(); + Self { + key_fields: value.key_fields.iter().map(|&f| f.into()).collect(), + filter: Some(filter), + } + } +} + +impl TryFrom for AssertUniqueKeys { + type Error = Error; + + fn try_from(message: pb::AssertUniqueKeys) -> Result { + let filter = required(message.filter, "AssertUniqueKeys.filter")?; + Ok(Self { + key_fields: message + .key_fields + .into_iter() + .map(Ref::try_from) + .collect::>>()?, + filter: KeyExistenceFilter::try_from(&filter)?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::key_existence::FilterType; + use crate::transaction::action::test_support::{apply, backed_manifest}; + use crate::transaction::action::{ + Action, AddFragment, CompositeOperation, RemoveFragment, UserAction, + }; + + fn assertion(key_fields: Vec, hashes: &[u64]) -> Action { + Action::AssertUniqueKeys(AssertUniqueKeys { + key_fields, + filter: KeyExistenceFilter { + field_ids: Vec::new(), + filter: FilterType::ExactSet(hashes.iter().copied().collect()), + }, + }) + } + + fn append(local: u32) -> Action { + Action::AddFragment(AddFragment { + local, + physical_rows: 5, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }) + } + + fn compact(local: u32) -> Action { + let Action::AddFragment(mut fragment) = append(local) else { + unreachable!() + }; + fragment.data_change = false; + Action::AddFragment(fragment) + } + + fn footprint(actions: Vec) -> Footprint { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + } + + #[test] + fn test_an_assertion_changes_nothing() { + let manifest = backed_manifest(); + let out = apply( + &manifest, + vec![assertion(vec![Ref::Committed(0)], &[1, 2, 3])], + ) + .unwrap(); + + assert_eq!(out.fragments, manifest.fragments); + assert_eq!(out.schema, manifest.schema); + } + + #[test] + fn test_asserting_over_no_key_column_is_rejected() { + let error = apply(&backed_manifest(), vec![assertion(vec![], &[1])]).unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("names no key column"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_asserting_over_a_field_that_is_not_there_is_rejected() { + let error = apply( + &backed_manifest(), + vec![assertion(vec![Ref::Committed(9)], &[1])], + ) + .unwrap_err(); + + assert!( + error.to_string().contains("not in the schema"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_inserts_with_disjoint_keys_do_not_conflict() { + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1, 2])]); + let theirs = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[3, 4])]); + + assert!(!ours.conflicts_with(&theirs)); + assert!(!theirs.conflicts_with(&ours)); + } + + #[test] + fn test_inserts_sharing_a_key_conflict() { + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1, 2])]); + let theirs = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[2, 3])]); + + assert!(ours.conflicts_with(&theirs)); + assert!(theirs.conflicts_with(&ours)); + } + + #[test] + fn test_assertions_over_different_key_columns_conflict() { + // Two filters over different columns hash different values, so neither + // says anything about the other's keys. + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1])]); + let theirs = footprint(vec![append(0), assertion(vec![Ref::Committed(1)], &[2])]); + + assert!(ours.conflicts_with(&theirs)); + } + + #[test] + fn test_an_assertion_conflicts_with_an_unqualified_insert() { + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1])]); + let plain_append = footprint(vec![append(0)]); + + assert!(ours.conflicts_with(&plain_append)); + assert!(plain_append.conflicts_with(&ours)); + } + + #[test] + fn test_two_plain_appends_still_do_not_conflict() { + assert!(!footprint(vec![append(0)]).conflicts_with(&footprint(vec![append(0)]))); + } + + #[test] + fn test_an_assertion_ignores_a_concurrent_change_that_inserts_no_rows() { + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1])]); + let removal = footprint(vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(3), + data_change: true, + })]); + + assert!(!ours.conflicts_with(&removal)); + } + + #[test] + fn test_a_compaction_is_not_an_insert() { + // Compaction rewrites rows that are already there, so it cannot have + // introduced a key the assertion would have to rule out. + let ours = footprint(vec![append(0), assertion(vec![Ref::Committed(0)], &[1])]); + let compaction = footprint(vec![compact(0)]); + + assert!(!ours.conflicts_with(&compaction)); + } +} diff --git a/rust/lance-table/src/transaction/action/config_update.rs b/rust/lance-table/src/transaction/action/config_update.rs new file mode 100644 index 00000000000..9ef57f506ad --- /dev/null +++ b/rust/lance-table/src/transaction/action/config_update.rs @@ -0,0 +1,496 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Apply config and metadata updates. + +use super::apply::ApplyState; +use super::proto::required; +use super::{ConfigMap, Footprint, Ref}; +use crate::format::pb; +use crate::transaction::UpdateMap; +use crate::transaction::update_map::apply_update_map; +use lance_core::datatypes::{ + Field, LANCE_UNENFORCED_CLUSTERING_KEY_POSITION, LANCE_UNENFORCED_PRIMARY_KEY, + LANCE_UNENFORCED_PRIMARY_KEY_POSITION, +}; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Apply config and metadata updates. +/// +/// Each of the four string maps a manifest carries is edited by an +/// [`UpdateMap`]: a list of keys to set or delete, or a wholesale replacement. +/// Absent means "leave this map alone", which is why every field is optional. +/// +/// This is reference-stable rather than a delta -- config keys and field ids are +/// stable coordinates -- so two operations editing different keys commute. +#[derive(Debug, Clone, PartialEq, DeepSizeOf, Default)] +pub struct ConfigUpdate { + /// Dataset config. + pub config: Option, + /// Table metadata. + pub table_metadata: Option, + /// Schema-level metadata. + pub schema_metadata: Option, + /// Per-field metadata, in application order. Keyed by [`Ref`] so a field + /// minted earlier in the same operation can be given metadata. + pub field_metadata: Vec, +} + +/// One field's metadata within a [`ConfigUpdate`]. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct FieldMetadataUpdate { + pub field: Ref, + pub updates: UpdateMap, +} + +impl ConfigUpdate { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + if let Some(updates) = &self.config { + apply_update_map(state.config_mut(), updates); + } + if let Some(updates) = &self.table_metadata { + apply_update_map(state.table_metadata_mut(), updates); + } + if let Some(updates) = &self.schema_metadata { + apply_update_map(&mut state.schema_mut().metadata, updates); + } + if self.field_metadata.is_empty() { + return Ok(()); + } + + // The unenforced primary and clustering keys are reserved schema + // properties: each is immutable once set, and its reserved keys cannot + // be written with an invalid value. Capture what they were before the + // updates land so a violation can be rejected below. This runs on every + // apply, including a conflict rebase, so it also catches the + // concurrent-writer race. + let primary_key_before = unenforced_primary_key(state); + let clustering_key_before = unenforced_clustering_key(state); + let writes_primary_key = self.writes_any(&[ + LANCE_UNENFORCED_PRIMARY_KEY, + LANCE_UNENFORCED_PRIMARY_KEY_POSITION, + ]); + let writes_clustering_key = self.writes_any(&[LANCE_UNENFORCED_CLUSTERING_KEY_POSITION]); + + for update in &self.field_metadata { + let field_id = state.resolve_field(update.field)?; + let field = state + .schema_mut() + .field_by_id_mut(field_id) + .ok_or_else(|| { + Error::invalid_input(format!( + "ConfigUpdate names field {field_id}, which does not exist" + )) + })?; + apply_update_map(&mut field.metadata, &update.updates); + refresh_reserved_key_positions(field); + } + + reject_reserved_key_change( + "primary", + &primary_key_before, + &unenforced_primary_key(state), + writes_primary_key, + )?; + reject_reserved_key_change( + "clustering", + &clustering_key_before, + &unenforced_clustering_key(state), + writes_clustering_key, + ) + } + + /// The config and metadata maps hold no row values. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// The keys this update names, or the whole map when it replaces one. A + /// field's metadata belongs to the field, so dropping the field also + /// collides with an update to it. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + for (map, update) in [ + (ConfigMap::Config, &self.config), + (ConfigMap::TableMetadata, &self.table_metadata), + (ConfigMap::SchemaMetadata, &self.schema_metadata), + ] { + if let Some(update) = update { + footprint.add_map_update(map, update); + } + } + for update in &self.field_metadata { + // A field minted in this operation has no committed id, so no + // concurrent writer can be naming it. + if let Some(id) = update.field.committed() + && let Ok(id) = i32::try_from(id) + { + footprint.add_map_update(ConfigMap::Field(id), &update.updates); + } + } + } + + fn writes_any(&self, keys: &[&str]) -> bool { + self.field_metadata.iter().any(|update| { + update + .updates + .update_entries + .iter() + .any(|entry| keys.contains(&entry.key.as_str())) + }) + } +} + +fn unenforced_primary_key(state: &ApplyState) -> Vec { + state + .schema() + .unenforced_primary_key() + .iter() + .map(|field| field.id) + .collect() +} + +fn unenforced_clustering_key(state: &ApplyState) -> Vec { + state + .schema() + .unenforced_clustering_key() + .iter() + .map(|field| field.id) + .collect() +} + +/// A field caches its reserved-key positions alongside the metadata they are +/// parsed from, so the cache has to be rebuilt whenever the metadata changes. +fn refresh_reserved_key_positions(field: &mut Field) { + field.unenforced_primary_key_position = field + .metadata + .get(LANCE_UNENFORCED_PRIMARY_KEY_POSITION) + .and_then(|value| value.parse::().ok()) + .or_else(|| { + field + .metadata + .get(LANCE_UNENFORCED_PRIMARY_KEY) + .filter(|value| matches!(value.to_lowercase().as_str(), "true" | "1" | "yes")) + .map(|_| 0) + }); + field.unenforced_clustering_key_position = field + .metadata + .get(LANCE_UNENFORCED_CLUSTERING_KEY_POSITION) + .and_then(|value| value.parse::().ok()); +} + +fn reject_reserved_key_change( + which: &str, + before: &[i32], + after: &[i32], + writes_reserved_key: bool, +) -> Result<()> { + if !before.is_empty() { + if writes_reserved_key || after != before { + return Err(Error::invalid_input(format!( + "the unenforced {which} key is a reserved key and cannot be changed once set" + ))); + } + } else if writes_reserved_key && after.is_empty() { + // A reserved key was written but installed no valid key, e.g. a + // non-marker flag value or a non-numeric position. + return Err(Error::invalid_input(format!( + "the unenforced {which} key is a reserved key and cannot be set to an invalid value" + ))); + } + Ok(()) +} + +impl From<&ConfigUpdate> for pb::ConfigUpdate { + fn from(value: &ConfigUpdate) -> Self { + Self { + config: value.config.as_ref().map(pb::UpdateMap::from), + table_metadata: value.table_metadata.as_ref().map(pb::UpdateMap::from), + schema_metadata: value.schema_metadata.as_ref().map(pb::UpdateMap::from), + field_metadata: value + .field_metadata + .iter() + .map(|update| pb::config_update::FieldMetadata { + field: Some(update.field.into()), + updates: Some(pb::UpdateMap::from(&update.updates)), + }) + .collect(), + } + } +} + +impl TryFrom for ConfigUpdate { + type Error = Error; + + fn try_from(message: pb::ConfigUpdate) -> Result { + Ok(Self { + config: message.config.as_ref().map(UpdateMap::from), + table_metadata: message.table_metadata.as_ref().map(UpdateMap::from), + schema_metadata: message.schema_metadata.as_ref().map(UpdateMap::from), + field_metadata: message + .field_metadata + .into_iter() + .map(|update| { + Ok(FieldMetadataUpdate { + field: required(update.field, "ConfigUpdate.field_metadata.field")? + .try_into()?, + updates: UpdateMap::from(&required( + update.updates, + "ConfigUpdate.field_metadata.updates", + )?), + }) + }) + .collect::>>()?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::test_support::{added_field, apply, backed_manifest}; + use crate::transaction::action::{Action, AddField, CompositeOperation, Footprint, UserAction}; + use crate::transaction::update_map::UpdateMapEntry; + + fn merge(entries: &[(&str, Option<&str>)]) -> UpdateMap { + UpdateMap { + update_entries: entries.iter().map(|entry| (*entry).into()).collect(), + replace: false, + } + } + + fn replace(entries: &[(&str, &str)]) -> UpdateMap { + UpdateMap { + update_entries: entries + .iter() + .map(|(key, value)| UpdateMapEntry { + key: (*key).into(), + value: Some((*value).into()), + }) + .collect(), + replace: true, + } + } + + fn footprint(actions: Vec) -> Footprint { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + } + + #[test] + fn test_config_update_merges_and_deletes() { + let mut manifest = backed_manifest(); + manifest.config.insert("keep".into(), "yes".into()); + manifest.config.insert("drop".into(), "yes".into()); + + let next = apply( + &manifest, + vec![Action::ConfigUpdate(ConfigUpdate { + config: Some(merge(&[("drop", None), ("added", Some("1"))])), + ..Default::default() + })], + ) + .unwrap(); + + assert_eq!(next.config.get("keep"), Some(&"yes".to_string())); + assert_eq!(next.config.get("added"), Some(&"1".to_string())); + assert!(!next.config.contains_key("drop")); + } + + #[test] + fn test_config_update_replaces_a_whole_map() { + let mut manifest = backed_manifest(); + manifest.table_metadata.insert("old".into(), "yes".into()); + + let next = apply( + &manifest, + vec![Action::ConfigUpdate(ConfigUpdate { + table_metadata: Some(replace(&[("new", "1")])), + ..Default::default() + })], + ) + .unwrap(); + + assert!(!next.table_metadata.contains_key("old")); + assert_eq!(next.table_metadata.get("new"), Some(&"1".to_string())); + } + + #[test] + fn test_config_update_edits_schema_and_field_metadata() { + let next = apply( + &backed_manifest(), + vec![Action::ConfigUpdate(ConfigUpdate { + schema_metadata: Some(merge(&[("schema", Some("yes"))])), + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Committed(0), + updates: merge(&[("comment", Some("the id"))]), + }], + ..Default::default() + })], + ) + .unwrap(); + + assert_eq!(next.schema.metadata.get("schema"), Some(&"yes".to_string())); + let field = next.schema.field_by_id(0).unwrap(); + assert_eq!(field.metadata.get("comment"), Some(&"the id".to_string())); + } + + #[test] + fn test_config_update_can_name_a_field_minted_in_the_same_operation() { + let next = apply( + &backed_manifest(), + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("fresh"), + }), + Action::ConfigUpdate(ConfigUpdate { + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Local(0), + updates: merge(&[("comment", Some("brand new"))]), + }], + ..Default::default() + }), + ], + ) + .unwrap(); + + let field = next.schema.field("fresh").unwrap(); + assert_eq!( + field.metadata.get("comment"), + Some(&"brand new".to_string()) + ); + } + + #[test] + fn test_config_update_rejects_a_missing_field() { + let error = apply( + &backed_manifest(), + vec![Action::ConfigUpdate(ConfigUpdate { + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Committed(7), + updates: merge(&[("comment", Some("nope"))]), + }], + ..Default::default() + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("field 7"), "{error}"); + } + + #[test] + fn test_config_update_sets_then_refuses_to_change_the_primary_key() { + let set_key = |field: u64| { + Action::ConfigUpdate(ConfigUpdate { + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Committed(field), + updates: merge(&[(LANCE_UNENFORCED_PRIMARY_KEY, Some("true"))]), + }], + ..Default::default() + }) + }; + + let next = apply(&backed_manifest(), vec![set_key(0)]).unwrap(); + assert_eq!( + next.schema + .unenforced_primary_key() + .iter() + .map(|field| field.id) + .collect::>(), + vec![0] + ); + + let error = apply(&next, vec![set_key(0)]).unwrap_err(); + assert!( + error.to_string().contains("cannot be changed once set"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_config_update_rejects_an_invalid_primary_key() { + let error = apply( + &backed_manifest(), + vec![Action::ConfigUpdate(ConfigUpdate { + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Committed(0), + updates: merge(&[( + LANCE_UNENFORCED_PRIMARY_KEY_POSITION, + Some("not a number"), + )]), + }], + ..Default::default() + })], + ) + .unwrap_err(); + + assert!( + error + .to_string() + .contains("cannot be set to an invalid value"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_edits_to_different_keys_commute() { + let update = |key: &str| { + vec![Action::ConfigUpdate(ConfigUpdate { + config: Some(merge(&[(key, Some("1"))])), + ..Default::default() + })] + }; + + assert!(!footprint(update("a")).conflicts_with(&footprint(update("b")))); + assert!(footprint(update("a")).conflicts_with(&footprint(update("a")))); + } + + #[test] + fn test_a_replacement_collides_with_any_edit_to_the_same_map() { + let replaced = footprint(vec![Action::ConfigUpdate(ConfigUpdate { + config: Some(replace(&[("a", "1")])), + ..Default::default() + })]); + let merged = footprint(vec![Action::ConfigUpdate(ConfigUpdate { + config: Some(merge(&[("untouched-by-name", Some("1"))])), + ..Default::default() + })]); + let other_map = footprint(vec![Action::ConfigUpdate(ConfigUpdate { + table_metadata: Some(merge(&[("a", Some("1"))])), + ..Default::default() + })]); + + assert!(replaced.conflicts_with(&merged)); + // Two clears of the same map name no key at all, and still collide. + assert!(replaced.conflicts_with(&replaced.clone())); + // A different map is a different coordinate space. + assert!(!replaced.conflicts_with(&other_map)); + } + + #[test] + fn test_dropping_a_field_collides_with_updating_its_metadata() { + use crate::transaction::action::DropField; + + let metadata = footprint(vec![Action::ConfigUpdate(ConfigUpdate { + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Committed(1), + updates: merge(&[("comment", Some("x"))]), + }], + ..Default::default() + })]); + let dropped = footprint(vec![Action::DropField(DropField { + field: Ref::Committed(1), + })]); + let other = footprint(vec![Action::DropField(DropField { + field: Ref::Committed(2), + })]); + + assert!(dropped.conflicts_with(&metadata)); + assert!(metadata.conflicts_with(&dropped)); + assert!(!other.conflicts_with(&metadata)); + } +} diff --git a/rust/lance-table/src/transaction/action/drop_field.rs b/rust/lance-table/src/transaction/action/drop_field.rs new file mode 100644 index 00000000000..218d027cfd4 --- /dev/null +++ b/rust/lance-table/src/transaction/action/drop_field.rs @@ -0,0 +1,291 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Remove a field from the schema. + +use super::apply::{ApplyState, TOMBSTONED_FIELD}; +use super::proto::required; +use super::{Footprint, Ref}; +use crate::format::pb; +use lance_core::datatypes::Field; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; +use std::collections::HashSet; + +/// Remove a field from the schema. +/// +/// The field's descendants go with it, since a struct's children cannot outlive +/// it. At apply, any data file left backing no live field is dropped, and any +/// index over a removed field is discarded. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct DropField { + pub field: Ref, +} + +impl DropField { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let field_id = state.resolve_field(self.field)?; + let field = state.schema().field_by_id(field_id).ok_or_else(|| { + Error::invalid_input(format!( + "DropField names field {field_id}, which does not exist" + )) + })?; + // A struct's children cannot outlive it, so the whole subtree goes. + let mut dropped = HashSet::new(); + collect_subtree_ids(field, &mut dropped); + remove_field(&mut state.schema_mut().fields, field_id); + + // The fields are gone from the schema, so the slots that backed them in + // each data file are dead. Tombstoning rather than rewriting the field + // list keeps a file's remaining columns at the positions they were + // written at; a file left with nothing live is pruned during + // normalization. + for fragment in state.fragments_mut() { + for file in fragment.files.iter_mut() { + if !file.fields.iter().any(|id| dropped.contains(id)) { + continue; + } + let fields = file + .fields + .iter() + .map(|id| { + if dropped.contains(id) { + TOMBSTONED_FIELD + } else { + *id + } + }) + .collect::>(); + file.fields = fields.into(); + } + + let overlaid: Vec = dropped + .iter() + .filter_map(|id| u32::try_from(*id).ok()) + .collect(); + crate::format::overlay::tombstone_overlay_fields(&mut fragment.overlays, &overlaid); + } + + // Indices over a field that no longer exists are discarded wholesale by + // `retain_relevant_indices`, so there is nothing to record here. + Ok(()) + } + + /// Dropping a field discards every value it held. + pub(super) fn is_data_change(&self) -> bool { + true + } + + /// The field's definition and all of its data, which cannot be enumerated, + /// so the removal is recorded as such and matched by field id. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.remove_field(self.field); + } +} + +fn collect_subtree_ids(field: &Field, out: &mut HashSet) { + out.insert(field.id); + for child in &field.children { + collect_subtree_ids(child, out); + } +} + +/// Remove the field with `field_id` from `fields`, at whatever depth it sits. +fn remove_field(fields: &mut Vec, field_id: i32) { + let before = fields.len(); + fields.retain(|field| field.id != field_id); + if fields.len() != before { + return; + } + for field in fields.iter_mut() { + remove_field(&mut field.children, field_id); + } +} + +impl From<&DropField> for pb::DropField { + fn from(value: &DropField) -> Self { + Self { + field: Some(value.field.into()), + } + } +} + +impl TryFrom for DropField { + type Error = Error; + + fn try_from(message: pb::DropField) -> Result { + Ok(Self { + field: required(message.field, "DropField.field")?.try_into()?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::action::test_support::{ + added_field, apply, apply_with_indices, backed_manifest, + }; + use crate::transaction::action::{Action, AddField}; + use crate::transaction::test_support::sample_index_metadata; + use arrow_schema::{DataType, Field as ArrowField}; + use lance_file::version::ConcreteFileVersion; + use std::sync::Arc; + + #[test] + fn test_drop_field_removes_it_and_its_data() { + let (next, indices) = apply_with_indices( + &backed_manifest(), + vec![Action::DropField(DropField { + field: Ref::Committed(0), + })], + vec![sample_index_metadata("idx")], + ) + .unwrap(); + + assert!(next.schema.field_by_id(0).is_none()); + // The file backed only the dropped field, so nothing is left of it. + assert!(next.fragments[0].files.is_empty()); + // An index over a field that no longer exists is discarded outright. + assert!(indices.is_empty()); + } + + #[test] + fn test_drop_field_keeps_a_file_with_a_surviving_field() { + let mut manifest = backed_manifest(); + let mut schema_field = added_field("keep"); + schema_field.id = 1; + manifest.schema.fields.push(schema_field); + let mut fragment = manifest.fragments[0].clone(); + fragment.files[0] = DataFile::new( + "data/0.lance", + vec![0, 1], + vec![0, 1], + ConcreteFileVersion::V2_0, + None, + None, + ); + manifest.fragments = Arc::new(vec![fragment]); + + let next = apply( + &manifest, + vec![Action::DropField(DropField { + field: Ref::Committed(0), + })], + ) + .unwrap(); + + assert!(next.schema.field_by_id(0).is_none()); + assert!(next.schema.field_by_id(1).is_some()); + // The surviving field stays at the position it was written at. + assert_eq!( + next.fragments[0].files[0].fields.as_ref(), + &[TOMBSTONED_FIELD, 1] + ); + } + + #[test] + fn test_drop_field_takes_the_whole_subtree() { + let mut manifest = backed_manifest(); + let mut parent = Field::try_from(ArrowField::new("parent", DataType::Int32, true)).unwrap(); + parent.id = 1; + let mut child = added_field("child"); + child.id = 2; + child.parent_id = 1; + parent.children.push(child); + manifest.schema.fields.push(parent); + + let next = apply( + &manifest, + vec![Action::DropField(DropField { + field: Ref::Committed(1), + })], + ) + .unwrap(); + + assert!(next.schema.field_by_id(1).is_none()); + assert!( + next.schema.field_by_id(2).is_none(), + "a struct's children cannot outlive it" + ); + } + + #[test] + fn test_drop_field_rejects_a_missing_field() { + let error = apply( + &backed_manifest(), + vec![Action::DropField(DropField { + field: Ref::Committed(7), + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("field 7"), "{error}"); + } + + #[test] + fn test_drop_field_then_add_a_field_reuses_no_id() { + let next = apply( + &backed_manifest(), + vec![ + Action::DropField(DropField { + field: Ref::Committed(0), + }), + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("replacement"), + }), + ], + ) + .unwrap(); + + // Field ids come from a monotonic counter, never from the freed id -- + // an old data file naming id 0 must not be read as the new field. + let ids = next + .schema + .fields_pre_order() + .map(|field| field.id) + .collect::>(); + assert_eq!(ids, vec![1]); + } + + #[test] + fn test_drop_field_rejects_a_field_id_out_of_range() { + let error = apply( + &backed_manifest(), + vec![Action::DropField(DropField { + field: Ref::Committed(u64::from(u32::MAX) + 1), + })], + ) + .unwrap_err(); + assert!( + error.to_string().contains("out of range"), + "unexpected message: {error}" + ); + } + + #[test] + fn test_drop_field_can_name_a_field_minted_in_the_same_operation() { + // A squash of "add column" and "drop column" lowers to exactly this, + // and has no committed id to name the field by. + let next = apply( + &backed_manifest(), + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("transient"), + }), + Action::DropField(DropField { + field: Ref::Local(0), + }), + ], + ) + .unwrap(); + + assert!(next.schema.field("transient").is_none()); + } +} diff --git a/rust/lance-table/src/transaction/action/footprint.rs b/rust/lance-table/src/transaction/action/footprint.rs new file mode 100644 index 00000000000..c904960a79a --- /dev/null +++ b/rust/lance-table/src/transaction/action/footprint.rs @@ -0,0 +1,519 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! The set of coordinates an action set writes. +//! +//! Two concurrent action sets can both commit when neither writes anything the +//! other writes. This is a structural test over coordinates rather than a +//! matrix over operation pairs, so it stays a single rule as the vocabulary +//! grows -- adding an action means saying which coordinates it writes, not +//! extending an N-by-N table. +//! +//! Footprints are derived from the actions at conflict time and never +//! serialized. A writer cannot pin down what a reader considers a conflict, and +//! the rule can be tightened in a later release without a format change. +//! +//! Which coordinates an action writes is decided by that action, in its own +//! module. This module holds the coordinate space and the comparison. + +use super::{CompositeOperation, Ref}; +use crate::format::key_existence::KeyExistenceFilter; +use crate::transaction::UpdateMap; +use std::collections::HashSet; +use uuid::Uuid; + +/// One thing an action set writes. +/// +/// Only committed coordinates appear. A minted fragment, field, or base has no +/// id in the read version, so no concurrent writer can be naming the same +/// thing; relocation re-resolves it against whatever version wins. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum Coordinate { + /// Whether a committed fragment is still part of the dataset. + FragmentExistence(u64), + /// A committed fragment's deletion file. + FragmentDeletions(u64), + /// A committed fragment's per-row version sequences. + FragmentRowVersions(u64), + /// The data backing one field within one committed fragment. + FieldData { fragment: u64, field: i32 }, + /// A field's definition in the schema. + FieldDefinition(i32), + /// A base path's name, which the manifest requires to be unique. + BaseName(Option), + /// A base path's location, which the manifest requires to be unique. + BaseLocation(String), + /// One key in one of the manifest's string maps. + ConfigEntry { map: ConfigMap, key: String }, + /// A logical index, by name. Segments are named by uuid, which the writer + /// picks and no one else can guess, so the name is the only index + /// coordinate two writers can arrive at independently. + IndexName(String), + /// One index segment, by uuid. + IndexSegment(Uuid), +} + +/// One of the string maps a manifest carries. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ConfigMap { + /// Dataset config. + Config, + /// Table metadata. + TableMetadata, + /// Schema-level metadata. + SchemaMetadata, + /// One field's metadata, by field id. + Field(i32), +} + +impl Coordinate { + /// The fragment this coordinate lives in, if it is fragment-scoped. + fn fragment(&self) -> Option { + match self { + Self::FragmentExistence(id) + | Self::FragmentDeletions(id) + | Self::FragmentRowVersions(id) => Some(*id), + Self::FieldData { fragment, .. } => Some(*fragment), + Self::FieldDefinition(_) + | Self::BaseName(_) + | Self::BaseLocation(_) + | Self::ConfigEntry { .. } + | Self::IndexName(_) + | Self::IndexSegment(_) => None, + } + } + + /// The field this coordinate belongs to, if it is field-scoped. + fn field(&self) -> Option { + match self { + Self::FieldData { field, .. } => Some(*field), + Self::FieldDefinition(id) => Some(*id), + // A field's metadata goes with the field, so dropping the field + // writes over a concurrent update to its metadata. + Self::ConfigEntry { + map: ConfigMap::Field(id), + .. + } => Some(*id), + Self::FragmentExistence(_) + | Self::FragmentDeletions(_) + | Self::FragmentRowVersions(_) + | Self::BaseName(_) + | Self::BaseLocation(_) + | Self::ConfigEntry { .. } + | Self::IndexName(_) + | Self::IndexSegment(_) => None, + } + } + + /// The string map this coordinate is a key in, if it is one. + fn config_map(&self) -> Option<&ConfigMap> { + match self { + Self::ConfigEntry { map, .. } => Some(map), + _ => None, + } + } +} + +/// Everything an action set writes, in one set. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct Footprint { + writes: HashSet, + /// Fragments this set removes outright. Removing a fragment writes every + /// coordinate inside it, which cannot be enumerated, so it is tracked + /// separately and matched against the other set by fragment id. + removed_fragments: HashSet, + /// Fields this set drops from the schema. Like a fragment removal, this + /// writes every coordinate belonging to the field -- its definition and its + /// data in every fragment -- so it is matched by field id. + /// + /// Only the named field, not its descendants: a footprint has no schema to + /// expand a struct with. A concurrent write to a child of a dropped struct + /// is therefore not caught here and fails when it is applied against the + /// version where the child no longer exists. + removed_fields: HashSet, + /// Fragments this set needs to still be there, without writing anything + /// exclusive inside them. An overlay is the case this exists for: two + /// concurrent overlays over the same cells both land and the newer one + /// wins, so they must not collide with each other -- but neither survives a + /// concurrent writer dropping the fragment out from under them. + required_fragments: HashSet, + /// String maps this set replaces outright rather than merging into. Like a + /// fragment removal, this writes every key in the map, including keys it + /// does not name, so it is matched by map rather than by key. + replaced_maps: HashSet, + /// Whether this set rewrites the table wholesale. Such a set writes every + /// coordinate there is, including ones a concurrent set would only mint, so + /// it is tracked as a flag rather than enumerated. + exclusive: bool, + /// Whether this set brings rows into the dataset that were not there + /// before. Coordinates cannot answer what the key assertions ask: two + /// writers inserting the same key write it into fragments of their own, so + /// their coordinates stay disjoint however badly the keys collide. Key + /// uniqueness is a claim about values, not about structure, so it is + /// checked by comparing this flag against the filters below. + /// + /// Set by any [`AddFragment`](super::AddFragment) that is a data change, + /// which over-approximates: the rows a merge insert updates arrive in a new + /// fragment too, and those carry no new key. The cost is a conflict between + /// two writers who only ever touched keys that were already there. + inserts_rows: bool, + /// The unique-key preconditions this set carries, one per + /// [`AssertUniqueKeys`](super::AssertUniqueKeys). + key_assertions: Vec, +} + +/// A claim about which keys an action set inserts, and over which columns. +#[derive(Debug, Clone, PartialEq)] +struct KeyAssertion { + key_fields: Vec, + filter: KeyExistenceFilter, +} + +impl Footprint { + pub fn conflicts_with(&self, other: &Self) -> bool { + // A wholesale rewrite leaves nothing for a concurrent set to land on -- + // not even an append, whose rows the reset would discard or resurrect + // depending on which commit won. + if self.exclusive || other.exclusive { + return true; + } + if !self.writes.is_disjoint(&other.writes) { + return true; + } + // Two sets replacing the same map collide even when neither names a + // key, since clearing a map is a replacement with no entries. + if !self.replaced_maps.is_disjoint(&other.replaced_maps) { + return true; + } + if self.key_assertion_violated_by(other) || other.key_assertion_violated_by(self) { + return true; + } + self.removes_something_touched_by(other) || other.removes_something_touched_by(self) + } + + /// Whether `other` may have inserted a key this set asserts is not there. + /// + /// A set that asserts nothing has nothing to violate, and a set that + /// inserts no rows cannot have inserted a key. Otherwise the two are only + /// compatible if `other` says which keys it inserted, over the same columns, + /// and the two filters provably do not intersect. Anything less -- an + /// unqualified insert, different key columns, filters built with + /// incomparable parameters -- leaves the assertion unverifiable, which + /// counts as a conflict. + fn key_assertion_violated_by(&self, other: &Self) -> bool { + if self.key_assertions.is_empty() || !other.inserts_rows { + return false; + } + if other.key_assertions.is_empty() { + return true; + } + for ours in &self.key_assertions { + for theirs in &other.key_assertions { + if ours.key_fields != theirs.key_fields { + return true; + } + match ours.filter.intersects(&theirs.filter) { + Ok((false, _)) => {} + // Either the keys really do overlap, or the two filters were + // built with parameters that cannot be compared. Neither + // clears the assertion. + Ok((true, _)) | Err(_) => return true, + } + } + } + false + } + + /// Whether this set wipes out something -- a fragment, a field, a whole + /// string map -- that `other` also writes to or needs to still be there. + fn removes_something_touched_by(&self, other: &Self) -> bool { + if !self + .removed_fragments + .is_disjoint(&other.required_fragments) + { + return true; + } + other.writes.iter().any(|coordinate| { + coordinate + .fragment() + .is_some_and(|id| self.removed_fragments.contains(&id)) + || coordinate + .field() + .is_some_and(|id| self.removed_fields.contains(&id)) + || coordinate + .config_map() + .is_some_and(|map| self.replaced_maps.contains(map)) + }) + } + + pub(super) fn add(&mut self, coordinate: Coordinate) { + self.writes.insert(coordinate); + } + + /// The data of each field within `fragment`. A fragment or field minted in + /// this same operation records nothing: no concurrent writer can be naming + /// one. + pub(super) fn add_field_data(&mut self, fragment: Ref, fields: impl IntoIterator) { + let Some(fragment) = fragment.committed() else { + return; + }; + for field in fields.into_iter().filter_map(committed_field) { + self.add(Coordinate::FieldData { fragment, field }); + } + } + + /// A field's entry in the schema. + pub(super) fn add_field_definition(&mut self, field: Ref) { + if let Some(field) = committed_field(field) { + self.add(Coordinate::FieldDefinition(field)); + } + } + + /// Note that this set only works if `fragment` is still part of the dataset, + /// without claiming anything inside it. + pub(super) fn require_fragment(&mut self, fragment: u64) { + self.required_fragments.insert(fragment); + } + + /// Note that this set brings in rows that were not in the dataset before. + pub(super) fn insert_rows(&mut self) { + self.inserts_rows = true; + } + + /// Record a precondition that no concurrent commit inserted a colliding key. + pub(super) fn assert_unique_keys(&mut self, key_fields: Vec, filter: KeyExistenceFilter) { + self.key_assertions + .push(KeyAssertion { key_fields, filter }); + } + + pub(super) fn remove_fragment(&mut self, fragment: u64) { + self.add(Coordinate::FragmentExistence(fragment)); + self.removed_fragments.insert(fragment); + } + + /// Record an edit to one of the manifest's string maps: the keys it names, + /// or the whole map when it replaces rather than merges. + pub(super) fn add_map_update(&mut self, map: ConfigMap, update: &UpdateMap) { + if update.replace { + self.replaced_maps.insert(map); + return; + } + for entry in &update.update_entries { + self.add(Coordinate::ConfigEntry { + map: map.clone(), + key: entry.key.clone(), + }); + } + } + + /// Mark this set as rewriting the whole table, conflicting with any + /// concurrent set whatsoever. + pub(super) fn take_exclusive(&mut self) { + self.exclusive = true; + } + + pub(super) fn remove_field(&mut self, field: Ref) { + if let Some(field) = committed_field(field) { + self.add(Coordinate::FieldDefinition(field)); + self.removed_fields.insert(field); + } + } +} + +/// A field id a concurrent writer could also be naming, or `None` for a field +/// this operation mints, which no one else can see yet. +fn committed_field(reference: Ref) -> Option { + i32::try_from(reference.committed()?).ok() +} + +impl From<&CompositeOperation> for Footprint { + fn from(composite_operation: &CompositeOperation) -> Self { + let mut footprint = Self::default(); + for action in composite_operation.iter_actions() { + action.footprint(&mut footprint); + } + footprint + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::{BasePath, DataFile}; + use crate::transaction::action::{ + Action, AddBase, AddDataFile, AddField, AddFragment, AlterField, DropField, RemoveFragment, + SetDeletionFile, TombstoneFieldData, UserAction, + }; + use arrow_schema::{DataType, Field as ArrowField}; + use lance_core::datatypes::Field; + use lance_file::version::ConcreteFileVersion; + use rstest::rstest; + + fn footprint(actions: Vec) -> Footprint { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + } + + fn add_fragment(local: u32) -> Action { + Action::AddFragment(AddFragment { + local, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }) + } + + fn add_data_file(fragment: Ref, fields: &[i32]) -> Action { + Action::AddDataFile(AddDataFile { + fragment, + file: DataFile::new_unstarted("data/x.lance", ConcreteFileVersion::V2_0), + field_ids: fields + .iter() + .map(|field| Ref::Committed(*field as u64)) + .collect(), + data_change: true, + }) + } + + fn tombstone(fragment: u64, fields: &[i32]) -> Action { + Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(fragment), + field_ids: fields.iter().map(|id| Ref::Committed(*id as u64)).collect(), + data_change: true, + }) + } + + fn remove_fragment(fragment: u64) -> Action { + Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(fragment), + data_change: true, + }) + } + + fn set_deletion_file(fragment: u64) -> Action { + Action::SetDeletionFile(SetDeletionFile { + fragment, + deletion_file: None, + data_change: true, + }) + } + + fn add_base(local: u32, name: &str, path: &str) -> Action { + Action::AddBase(AddBase { + local, + base: BasePath::new(0, path.into(), Some(name.into()), false), + }) + } + + #[test] + fn test_minting_actions_write_nothing() { + let minting = footprint(vec![ + add_fragment(0), + Action::AddField(AddField { + local: 1, + parent: None, + def: Field::try_from(ArrowField::new("new", DataType::Int32, true)).unwrap(), + }), + add_data_file(Ref::Local(0), &[0]), + ]); + + // Two writers appending at the same time never collide. + assert!(!minting.conflicts_with(&minting.clone())); + } + + #[rstest] + #[case::same_field_in_same_fragment( + vec![tombstone(0, &[1])], + vec![add_data_file(Ref::Committed(0), &[1])], + true, + )] + #[case::different_fields_in_same_fragment( + vec![tombstone(0, &[1])], + vec![add_data_file(Ref::Committed(0), &[2])], + false, + )] + #[case::same_field_in_different_fragments( + vec![tombstone(0, &[1])], + vec![tombstone(1, &[1])], + false, + )] + #[case::deletions_do_not_collide_with_field_data( + vec![set_deletion_file(0)], + vec![tombstone(0, &[1])], + false, + )] + #[case::concurrent_deletes_of_one_fragment( + vec![set_deletion_file(0)], + vec![set_deletion_file(0)], + true, + )] + #[case::removal_swallows_the_whole_fragment( + vec![remove_fragment(0)], + vec![tombstone(0, &[1])], + true, + )] + #[case::removal_leaves_other_fragments_alone( + vec![remove_fragment(0)], + vec![tombstone(1, &[1])], + false, + )] + #[case::same_field_definition( + vec![Action::AlterField(AlterField { field: Ref::Committed(1), name: Some("a".into()), logical_type: None, nullable: None })], + vec![Action::AlterField(AlterField { field: Ref::Committed(1), name: None, logical_type: None, nullable: Some(true) })], + true, + )] + #[case::different_field_definitions( + vec![Action::AlterField(AlterField { field: Ref::Committed(1), name: None, logical_type: None, nullable: None })], + vec![Action::AlterField(AlterField { field: Ref::Committed(2), name: None, logical_type: None, nullable: None })], + false, + )] + #[case::dropping_a_field_collides_with_altering_it( + vec![Action::DropField(DropField { field: Ref::Committed(1) })], + vec![Action::AlterField(AlterField { field: Ref::Committed(1), name: None, logical_type: None, nullable: Some(true) })], + true, + )] + #[case::dropping_a_field_collides_with_rewriting_its_data( + vec![Action::DropField(DropField { field: Ref::Committed(1) })], + vec![tombstone(0, &[1])], + true, + )] + #[case::dropping_a_field_leaves_other_fields_alone( + vec![Action::DropField(DropField { field: Ref::Committed(1) })], + vec![tombstone(0, &[2])], + false, + )] + #[case::dropping_a_field_leaves_deletions_alone( + vec![Action::DropField(DropField { field: Ref::Committed(1) })], + vec![set_deletion_file(0)], + false, + )] + #[case::bases_with_the_same_name( + vec![add_base(0, "a", "s3://bucket/one")], + vec![add_base(0, "a", "s3://bucket/two")], + true, + )] + #[case::bases_with_the_same_location( + vec![add_base(0, "a", "s3://bucket/one")], + vec![add_base(0, "b", "s3://bucket/one")], + true, + )] + #[case::unrelated_bases( + vec![add_base(0, "a", "s3://bucket/one")], + vec![add_base(0, "b", "s3://bucket/two")], + false, + )] + fn test_conflicts( + #[case] ours: Vec, + #[case] theirs: Vec, + #[case] expected: bool, + ) { + let ours = footprint(ours); + let theirs = footprint(theirs); + assert_eq!(ours.conflicts_with(&theirs), expected); + // The relation has to hold whichever side is asking. + assert_eq!(theirs.conflicts_with(&ours), expected); + } +} diff --git a/rust/lance-table/src/transaction/action/proto.rs b/rust/lance-table/src/transaction/action/proto.rs new file mode 100644 index 00000000000..541f7f294a5 --- /dev/null +++ b/rust/lance-table/src/transaction/action/proto.rs @@ -0,0 +1,359 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! The envelope around the per-action protobuf encodings. +//! +//! Each action encodes itself, in its own module; this module carries the +//! [`Ref`], [`CompositeOperation`], and [`UserAction`] wrappers, the dispatch over +//! the `oneof`, and the helpers the per-action conversions share. +//! +//! Reading is fail-closed: an action this build does not recognize is an error, +//! never a silently skipped element. The commit path collects concurrent +//! transactions with `try_collect`, so a transaction carrying an unknown action +//! must abort the commit rather than be treated as a no-op. Every drafted action +//! is implemented, so an unrecognized one can only come from a newer Lance -- +//! which protobuf decodes as no variant at all, since it drops the field it does +//! not know. + +use super::{Action, CompositeOperation, Ref, UserAction}; +use crate::format::pb; +use lance_core::{Error, Result}; + +/// `data_change` is absent-means-true on the wire, so only the `false` case is +/// written out. +pub(super) fn data_change_to_wire(data_change: bool) -> Option { + (!data_change).then_some(false) +} + +pub(super) fn data_change_from_wire(data_change: Option) -> bool { + data_change.unwrap_or(true) +} + +pub(super) fn required(value: Option, what: &str) -> Result { + value.ok_or_else(|| Error::invalid_input(format!("{what} is required but was not set"))) +} + +impl From for pb::Ref { + fn from(value: Ref) -> Self { + let kind = match value { + Ref::Committed(id) => pb::r#ref::Kind::Committed(id), + Ref::Local(token) => pb::r#ref::Kind::Local(token), + }; + Self { kind: Some(kind) } + } +} + +impl TryFrom for Ref { + type Error = Error; + + fn try_from(message: pb::Ref) -> Result { + match message.kind { + Some(pb::r#ref::Kind::Committed(id)) => Ok(Self::Committed(id)), + Some(pb::r#ref::Kind::Local(token)) => Ok(Self::Local(token)), + None => Err(Error::invalid_input( + "a Ref in an action was empty; it must be either committed or local", + )), + } + } +} + +impl From<&CompositeOperation> for pb::CompositeOperation { + fn from(value: &CompositeOperation) -> Self { + Self { + // uuid and read_version mirror the enclosing Transaction and are + // stamped in by its conversion. + uuid: String::new(), + read_version: 0, + actions: value.actions.iter().map(pb::UserAction::from).collect(), + } + } +} + +impl TryFrom for CompositeOperation { + type Error = Error; + + fn try_from(message: pb::CompositeOperation) -> Result { + Ok(Self { + actions: message + .actions + .into_iter() + .map(UserAction::try_from) + .collect::>>()?, + }) + } +} + +impl From<&UserAction> for pb::UserAction { + fn from(value: &UserAction) -> Self { + Self { + description: value.description.clone(), + actions: value.actions.iter().map(pb::Action::from).collect(), + } + } +} + +impl TryFrom for UserAction { + type Error = Error; + + fn try_from(message: pb::UserAction) -> Result { + Ok(Self { + description: message.description, + actions: message + .actions + .into_iter() + .map(Action::try_from) + .collect::>>()?, + }) + } +} + +macro_rules! define_action_proto { + ($($variant:ident,)*) => { + impl From<&Action> for pb::Action { + fn from(value: &Action) -> Self { + let action = match value { + $(Action::$variant(action) => pb::action::Action::$variant(action.into()),)* + }; + Self { + action: Some(action), + } + } + } + + impl TryFrom for Action { + type Error = Error; + + fn try_from(message: pb::Action) -> Result { + match message.action { + $(Some(pb::action::Action::$variant(action)) => { + Ok(Self::$variant(action.try_into()?)) + })* + // An action written by a newer Lance decodes to no known + // variant, because protobuf drops the field it does not + // know. Reject rather than skip: silently dropping an + // action would apply a partial transaction. + None => Err(Error::not_supported( + "an Action in a user operation carried no change this version of Lance \ + understands; it was either empty or written by a newer version", + )), + } + } + } + }; +} + +for_each_action!(define_action_proto); + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::key_existence::{FilterType, KeyExistenceFilter}; + use crate::format::overlay::{DataOverlayFile, OverlayCoverage}; + use crate::format::{ + BasePath, DataFile, DeletionFile, DeletionFileType, IndexFile, RowIdMeta, pb, + }; + use crate::rowids::version::RowDatasetVersionMeta; + use crate::system_index::mem_wal::CompactedSsTable; + use crate::transaction::UpdateMap; + use crate::transaction::action::{ + AddBase, AddDataFile, AddField, AddFragment, AddIndexSegment, AddOverlays, + AdjustIndexCoverage, AlterField, AssertUniqueKeys, ConfigUpdate, DropField, + FieldMetadataUpdate, RefreshRowVersionMetadata, RemoveFragment, RemoveIndexSegment, + ReserveFragmentIds, ResetTable, SetDeletionFile, TombstoneFieldData, + UpdateCompactedSsTables, + }; + use arrow_schema::{DataType, Field as ArrowField}; + use chrono::DateTime; + use lance_core::datatypes::Field; + use lance_file::version::ConcreteFileVersion; + use roaring::RoaringBitmap; + use std::sync::Arc; + use uuid::Uuid; + + fn sample_data_file() -> DataFile { + DataFile::new_unstarted("data/1.lance", ConcreteFileVersion::V2_0) + } + + fn all_actions() -> Vec { + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: Some(RowIdMeta::Inline(vec![1, 2, 3].into())), + last_updated_at_version_meta: Some(RowDatasetVersionMeta::Inline(Arc::from( + [4u8, 5].as_slice(), + ))), + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: sample_data_file(), + field_ids: vec![Ref::Committed(1), Ref::Local(3)], + data_change: false, + }), + Action::AddField(AddField { + local: 3, + parent: Some(Ref::Committed(1)), + def: Field::try_from(ArrowField::new("added", DataType::Int32, true)).unwrap(), + }), + Action::AddBase(AddBase { + local: 1, + base: BasePath::new(0, "s3://bucket/x".into(), Some("other".into()), false), + }), + Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(4), + field_ids: vec![Ref::Committed(7), Ref::Committed(8)], + data_change: true, + }), + Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(5), + data_change: true, + }), + Action::SetDeletionFile(SetDeletionFile { + fragment: 6, + deletion_file: Some(DeletionFile { + read_version: 3, + id: 9, + file_type: DeletionFileType::Bitmap, + num_deleted_rows: Some(4), + base_id: None, + }), + data_change: true, + }), + Action::AddOverlays(AddOverlays { + fragment: Ref::Committed(6), + overlays: vec![DataOverlayFile { + data_file: sample_data_file(), + coverage: OverlayCoverage::PerField(vec![Arc::new( + [1u32, 4].into_iter().collect::(), + )]), + committed_version: 11, + }], + data_change: true, + }), + Action::RefreshRowVersionMetadata(RefreshRowVersionMetadata { + fragment_ids: vec![4, 6], + }), + Action::AlterField(AlterField { + field: Ref::Committed(2), + name: Some("renamed".into()), + logical_type: Some("int64".into()), + nullable: Some(false), + }), + Action::DropField(DropField { + field: Ref::Committed(3), + }), + Action::AddIndexSegment(AddIndexSegment { + uuid: Uuid::from_u128(7), + name: "by_a".into(), + fields: vec![Ref::Committed(1), Ref::Local(3)], + index_details: Some(Arc::new(prost_types::Any { + type_url: "type.googleapis.com/lance.table.MemWalIndexDetails".into(), + value: vec![1, 2, 3], + })), + index_version: 2, + covered_fragments: Some(vec![Ref::Committed(4), Ref::Local(0)]), + files: vec![IndexFile { + path: "index.idx".into(), + size_bytes: 512, + }], + base: Some(Ref::Local(1)), + created_at: DateTime::from_timestamp_millis(1_700_000_000_000), + dataset_version: Some(3), + data_change: false, + }), + Action::RemoveIndexSegment(RemoveIndexSegment { + uuid: Uuid::from_u128(8), + data_change: true, + }), + Action::AdjustIndexCoverage(AdjustIndexCoverage { + uuid: Uuid::from_u128(9), + add_fragments: vec![Ref::Committed(1), Ref::Local(0)], + remove_fragments: vec![2, 3], + }), + Action::UpdateCompactedSsTables(UpdateCompactedSsTables { + compacted_sstables: vec![ + CompactedSsTable::new(Uuid::from_u128(10), 2), + CompactedSsTable::new(Uuid::from_u128(11), 5), + ], + }), + Action::AssertUniqueKeys(AssertUniqueKeys { + key_fields: vec![Ref::Committed(1), Ref::Local(3)], + filter: KeyExistenceFilter { + field_ids: Vec::new(), + filter: FilterType::ExactSet([7u64, 9].into_iter().collect()), + }, + }), + Action::ReserveFragmentIds(ReserveFragmentIds { count: 4 }), + Action::ResetTable(ResetTable), + Action::ConfigUpdate(ConfigUpdate { + config: Some(UpdateMap { + update_entries: vec![("a", "1").into(), ("b", None).into()], + replace: false, + }), + table_metadata: None, + schema_metadata: Some(UpdateMap { + update_entries: vec![("c", "2").into()], + replace: true, + }), + field_metadata: vec![FieldMetadataUpdate { + field: Ref::Local(3), + updates: UpdateMap { + update_entries: vec![("d", "3").into()], + replace: false, + }, + }], + }), + ] + } + + #[test] + fn test_composite_operation_round_trips() { + let operation = CompositeOperation::new(vec![ + UserAction::new("everything", all_actions()), + UserAction::new("nothing", vec![]), + ]); + + let message = pb::CompositeOperation::from(&operation); + let round_tripped = CompositeOperation::try_from(message).unwrap(); + + assert_eq!(round_tripped, operation); + } + + #[test] + fn test_data_change_is_absent_when_true() { + // Absent means "real change" on the wire, so the common case costs no + // bytes and an old field-less writer is read correctly. + let message = pb::RemoveFragment::from(&RemoveFragment { + fragment: Ref::Committed(1), + data_change: true, + }); + assert_eq!(message.data_change, None); + assert!(RemoveFragment::try_from(message).unwrap().data_change); + } + + #[test] + fn test_an_unrecognized_action_is_rejected() { + // An empty action and one a newer Lance wrote decode the same way: + // protobuf drops the field this build does not know. + let error = Action::try_from(pb::Action { action: None }).unwrap_err(); + assert!( + matches!(error, Error::NotSupported { .. }), + "expected NotSupported, got {error:?}" + ); + assert!( + error.to_string().contains("written by a newer version"), + "unexpected message: {error}" + ); + } + + #[test] + fn test_empty_ref_is_rejected() { + let error = Ref::try_from(pb::Ref { kind: None }).unwrap_err(); + assert!( + error.to_string().contains("committed or local"), + "unexpected message: {error}" + ); + } +} diff --git a/rust/lance-table/src/transaction/action/refresh_row_version_metadata.rs b/rust/lance-table/src/transaction/action/refresh_row_version_metadata.rs new file mode 100644 index 00000000000..06074d0934b --- /dev/null +++ b/rust/lance-table/src/transaction/action/refresh_row_version_metadata.rs @@ -0,0 +1,198 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Restamp the row-version metadata of fragments rewritten in place. + +use super::apply::ApplyState; +use super::{Coordinate, Footprint}; +use crate::format::pb; +use crate::rowids::version::refresh_row_latest_update_meta_for_full_frag_rewrite_cols; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Record that every row of these fragments was updated by this operation. +/// +/// Under stable row ids a fragment carries a `last_updated_at_version` sequence +/// per row. Rewriting a fragment's columns in place leaves the rows where they +/// are, so nothing else in the operation restates when they last changed; this +/// action does, mirroring the refresh a legacy `Merge` performs implicitly. +/// +/// `created_at_version` is deliberately untouched: the rows are the same rows, +/// and a row minted by this operation gets both stamps from the +/// [`AddFragment`](super::AddFragment) that minted it. +/// +/// Fragments are named by committed id. A fragment this operation minted has +/// nothing to restamp. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct RefreshRowVersionMetadata { + pub fragment_ids: Vec, +} + +impl RefreshRowVersionMetadata { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + if !self.fragment_ids.is_empty() && !state.uses_stable_row_ids() { + return Err(Error::invalid_input( + "RefreshRowVersionMetadata restamps the per-row version sequences, which only \ + exist on a dataset using stable row ids", + )); + } + let new_version = state.new_version(); + for fragment_id in &self.fragment_ids { + let fragment = state.fragment_mut(*fragment_id, "RefreshRowVersionMetadata")?; + refresh_row_latest_update_meta_for_full_frag_rewrite_cols(fragment, new_version)?; + } + Ok(()) + } + + /// The rows themselves are restated elsewhere in the operation -- by the + /// data files it writes -- so this is bookkeeping about that change, not a + /// change of its own. + pub(super) fn is_data_change(&self) -> bool { + false + } + + pub(super) fn footprint(&self, footprint: &mut Footprint) { + for fragment_id in &self.fragment_ids { + footprint.add(Coordinate::FragmentRowVersions(*fragment_id)); + } + } +} + +impl From<&RefreshRowVersionMetadata> for pb::RefreshRowVersionMetadata { + fn from(value: &RefreshRowVersionMetadata) -> Self { + Self { + fragment_ids: value.fragment_ids.clone(), + } + } +} + +impl TryFrom for RefreshRowVersionMetadata { + type Error = Error; + + fn try_from(message: pb::RefreshRowVersionMetadata) -> Result { + Ok(Self { + fragment_ids: message.fragment_ids, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::{DataFile, Fragment, RowIdMeta}; + use crate::rowids::{RowIdSequence, write_row_ids}; + use crate::transaction::action::test_support::{apply, backed_manifest}; + use crate::transaction::action::{Action, CompositeOperation, UserAction}; + use crate::transaction::test_support::make_stable_row_id_manifest; + use lance_file::version::ConcreteFileVersion; + use std::sync::Arc; + + fn refresh(fragment_ids: Vec) -> Action { + Action::RefreshRowVersionMetadata(RefreshRowVersionMetadata { fragment_ids }) + } + + /// A stable-row-id manifest whose fragment 0 has `rows` rows, all last + /// updated at version 1. + fn manifest_with_rows(rows: usize) -> crate::format::Manifest { + let row_ids = RowIdSequence::from((0..rows as u64).collect::>().as_slice()); + let mut fragment = Fragment { + id: 0, + files: vec![DataFile::new( + "data.lance", + vec![0], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + )], + overlays: vec![], + deletion_file: None, + row_id_meta: Some(RowIdMeta::Inline(write_row_ids(&row_ids).into())), + physical_rows: Some(rows), + last_updated_at_version_meta: None, + created_at_version_meta: None, + }; + refresh_row_latest_update_meta_for_full_frag_rewrite_cols(&mut fragment, 1).unwrap(); + make_stable_row_id_manifest(vec![fragment]) + } + + fn last_updated_versions(fragment: &Fragment) -> Vec { + let meta = fragment + .last_updated_at_version_meta + .as_ref() + .expect("the fragment should carry a last-updated sequence"); + let sequence = meta.load_sequence().unwrap(); + (0..fragment.physical_rows.unwrap()) + .map(|offset| sequence.version_at(offset).unwrap()) + .collect() + } + + #[test] + fn test_refresh_stamps_every_row_with_the_version_the_commit_produces() { + let manifest = manifest_with_rows(3); + let expected = manifest.version + 1; + + let out = apply(&manifest, vec![refresh(vec![0])]).unwrap(); + + assert_eq!(last_updated_versions(&out.fragments[0]), vec![expected; 3]); + } + + #[test] + fn test_refresh_leaves_the_created_at_sequence_alone() { + let mut manifest = manifest_with_rows(3); + let mut fragment = manifest.fragments[0].clone(); + fragment.created_at_version_meta = fragment.last_updated_at_version_meta.clone(); + let created_at = fragment.created_at_version_meta.clone(); + manifest.fragments = Arc::new(vec![fragment]); + + let out = apply(&manifest, vec![refresh(vec![0])]).unwrap(); + + assert_eq!(out.fragments[0].created_at_version_meta, created_at); + } + + #[test] + fn test_refreshing_no_fragments_is_allowed_on_any_dataset() { + // An empty list is what a translated operation produces when nothing was + // rewritten in place, so it must not depend on stable row ids. + apply(&backed_manifest(), vec![refresh(vec![])]).unwrap(); + } + + #[test] + fn test_refreshing_without_stable_row_ids_is_rejected() { + let error = apply(&backed_manifest(), vec![refresh(vec![0])]).unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("stable row ids"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_refreshing_a_fragment_that_is_not_there_is_rejected() { + let error = apply(&manifest_with_rows(3), vec![refresh(vec![7])]).unwrap_err(); + + assert!( + error + .to_string() + .contains("RefreshRowVersionMetadata targets fragment 7, which does not exist"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_two_writers_refreshing_the_same_fragment_conflict() { + let footprint = |actions| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + }; + + let ours = footprint(vec![refresh(vec![0])]); + let same = footprint(vec![refresh(vec![0, 1])]); + let other = footprint(vec![refresh(vec![1])]); + + assert!(ours.conflicts_with(&same)); + assert!(!ours.conflicts_with(&other)); + } +} diff --git a/rust/lance-table/src/transaction/action/remove_fragment.rs b/rust/lance-table/src/transaction/action/remove_fragment.rs new file mode 100644 index 00000000000..43cd41fdd89 --- /dev/null +++ b/rust/lance-table/src/transaction/action/remove_fragment.rs @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Remove a fragment entirely. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Footprint, Ref}; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Remove a fragment entirely -- every row deleted, or the fragment replaced by +/// compaction. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct RemoveFragment { + pub fragment: Ref, + /// See [`AddFragment::data_change`](super::AddFragment::data_change). + pub data_change: bool, +} + +impl RemoveFragment { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let fragment_id = state.resolve_fragment(self.fragment)?; + if !state.remove_fragment(fragment_id) { + return Err(Error::invalid_input(format!( + "RemoveFragment targets fragment {fragment_id}, which does not exist" + ))); + } + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// Every coordinate inside the fragment, which cannot be enumerated, so the + /// removal is recorded as such and matched by fragment id. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + if let Some(id) = self.fragment.committed() { + footprint.remove_fragment(id); + } + } +} + +impl From<&RemoveFragment> for pb::RemoveFragment { + fn from(value: &RemoveFragment) -> Self { + Self { + fragment: Some(value.fragment.into()), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for RemoveFragment { + type Error = Error; + + fn try_from(message: pb::RemoveFragment) -> Result { + Ok(Self { + fragment: required(message.fragment, "RemoveFragment.fragment")?.try_into()?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::test_support::{apply, backed_manifest}; + use crate::transaction::action::{Action, AddFragment}; + + #[test] + fn test_remove_fragment() { + let next = apply( + &backed_manifest(), + vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(0), + data_change: true, + })], + ) + .unwrap(); + + assert!(next.fragments.is_empty()); + } + + #[test] + fn test_remove_fragment_can_drop_one_minted_in_the_same_operation() { + let next = apply( + &backed_manifest(), + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::RemoveFragment(RemoveFragment { + fragment: Ref::Local(0), + data_change: true, + }), + ], + ) + .unwrap(); + + assert_eq!( + next.fragments.iter().map(|f| f.id).collect::>(), + vec![0] + ); + } + + #[test] + fn test_remove_fragment_rejects_a_missing_fragment() { + let error = apply( + &backed_manifest(), + vec![Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(7), + data_change: true, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("fragment 7"), "{error}"); + } +} diff --git a/rust/lance-table/src/transaction/action/remove_index_segment.rs b/rust/lance-table/src/transaction/action/remove_index_segment.rs new file mode 100644 index 00000000000..9cdae2646f7 --- /dev/null +++ b/rust/lance-table/src/transaction/action/remove_index_segment.rs @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Remove an index segment. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Coordinate, Footprint}; +use crate::format::pb; +use lance_core::deepsize::{Context, DeepSizeOf}; +use lance_core::{Error, Result}; +use uuid::Uuid; + +/// Remove an index segment. +/// +/// Dropping a whole logical index is one of these per segment carrying its +/// name, since the format knows only segments. Removing the last segment of an +/// index is what makes the index disappear. +#[derive(Debug, Clone, PartialEq)] +pub struct RemoveIndexSegment { + pub uuid: Uuid, + pub data_change: bool, +} + +impl RemoveIndexSegment { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + state.remove_index_segment(self.uuid) + } + + /// Left to the writer, for the same reason as + /// [`AddIndexSegment`](super::AddIndexSegment): an index is normally derived + /// state, but the MemWAL index holds rows a reader can see. + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// The segment, by uuid. Only another action naming the same segment + /// collides: a concurrent writer extending the same logical index is adding + /// a segment of its own, which this removal leaves alone. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::IndexSegment(self.uuid)); + } +} + +impl DeepSizeOf for RemoveIndexSegment { + fn deep_size_of_children(&self, context: &mut Context) -> usize { + self.uuid.as_bytes().deep_size_of_children(context) + } +} + +impl From<&RemoveIndexSegment> for pb::RemoveIndexSegment { + fn from(value: &RemoveIndexSegment) -> Self { + Self { + uuid: Some((&value.uuid).into()), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for RemoveIndexSegment { + type Error = Error; + + fn try_from(message: pb::RemoveIndexSegment) -> Result { + Ok(Self { + uuid: Uuid::try_from(&required(message.uuid, "RemoveIndexSegment.uuid")?)?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::test_support::{apply_with_indices, backed_manifest}; + use crate::transaction::action::{Action, CompositeOperation, Footprint, UserAction}; + use crate::transaction::test_support::sample_index_metadata; + + fn remove(uuid: Uuid) -> Action { + Action::RemoveIndexSegment(RemoveIndexSegment { + uuid, + data_change: false, + }) + } + + #[test] + fn test_remove_index_segment_drops_only_the_segment_it_names() { + let dropped = sample_index_metadata("by_a"); + let kept = sample_index_metadata("by_b"); + let dropped_uuid = dropped.uuid; + + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![remove(dropped_uuid)], + vec![dropped, kept.clone()], + ) + .unwrap(); + + assert_eq!(indices.len(), 1); + assert_eq!(indices[0].uuid, kept.uuid); + } + + #[test] + fn test_dropping_an_index_removes_each_of_its_segments() { + let first = sample_index_metadata("by_a"); + let second = sample_index_metadata("by_a"); + + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![remove(first.uuid), remove(second.uuid)], + vec![first, second], + ) + .unwrap(); + + assert!(indices.is_empty()); + } + + #[test] + fn test_removing_a_segment_that_is_not_there_is_rejected() { + let error = apply_with_indices( + &backed_manifest(), + vec![remove(Uuid::from_u128(1))], + vec![sample_index_metadata("by_a")], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("is not part of the dataset"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_a_segment_can_be_replaced_within_one_operation() { + use crate::transaction::action::{AddIndexSegment, Ref}; + + let old = sample_index_metadata("by_a"); + let new_uuid = Uuid::from_u128(2); + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![ + remove(old.uuid), + Action::AddIndexSegment(AddIndexSegment { + uuid: new_uuid, + name: "by_a".into(), + fields: vec![Ref::Committed(0)], + index_details: None, + index_version: 1, + covered_fragments: Some(vec![Ref::Committed(0)]), + files: Vec::new(), + base: None, + created_at: None, + dataset_version: None, + data_change: false, + }), + ], + vec![old], + ) + .unwrap(); + + assert_eq!(indices.len(), 1); + assert_eq!(indices[0].uuid, new_uuid); + } + + #[test] + fn test_two_writers_removing_the_same_segment_conflict() { + let footprint = |actions| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + }; + + let uuid = Uuid::from_u128(1); + let ours = footprint(vec![remove(uuid)]); + let same = footprint(vec![remove(uuid)]); + let other = footprint(vec![remove(Uuid::from_u128(2))]); + + assert!(ours.conflicts_with(&same)); + assert!(!ours.conflicts_with(&other)); + } +} diff --git a/rust/lance-table/src/transaction/action/reserve_fragment_ids.rs b/rust/lance-table/src/transaction/action/reserve_fragment_ids.rs new file mode 100644 index 00000000000..3e4697eb6e3 --- /dev/null +++ b/rust/lance-table/src/transaction/action/reserve_fragment_ids.rs @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Reserve a range of fragment ids for a later writer. + +use super::Footprint; +use super::apply::ApplyState; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Reserve a contiguous range of fragment ids from the counter, for a later +/// (possibly distributed) writer to populate. +/// +/// The range starts wherever the counter stands when this action is applied, so +/// the reserving writer learns which ids it got by reading the committed +/// manifest's high-water mark: the range is the `count` ids ending there. +/// Fragments written against the range name those ids as +/// [`Ref::Committed`](super::Ref::Committed). +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct ReserveFragmentIds { + pub count: u32, +} + +impl ReserveFragmentIds { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + state.reserve_fragment_ids(self.count); + Ok(()) + } + + /// A reserved id backs no rows until something is written to it. + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// Nothing. Ids come off a monotonic counter, so two operations reserving + /// at once get disjoint ranges rather than colliding. + pub(super) fn footprint(&self, _footprint: &mut Footprint) {} +} + +impl From<&ReserveFragmentIds> for pb::ReserveFragmentIds { + fn from(value: &ReserveFragmentIds) -> Self { + Self { count: value.count } + } +} + +impl TryFrom for ReserveFragmentIds { + type Error = Error; + + fn try_from(message: pb::ReserveFragmentIds) -> Result { + Ok(Self { + count: message.count, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transaction::action::test_support::{apply, backed_manifest}; + use crate::transaction::action::{Action, AddFragment}; + + fn reserve(count: u32) -> Action { + Action::ReserveFragmentIds(ReserveFragmentIds { count }) + } + + fn add_fragment(local: u32) -> Action { + Action::AddFragment(AddFragment { + local, + physical_rows: 10, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }) + } + + #[test] + fn test_reserving_raises_the_high_water_mark_without_adding_fragments() { + let manifest = backed_manifest(); + assert_eq!(manifest.max_fragment_id(), Some(0)); + + let next = apply(&manifest, vec![reserve(3)]).unwrap(); + + // Ids 1, 2 and 3 are now spoken for, but no fragment exists for them. + assert_eq!(next.max_fragment_id(), Some(3)); + assert_eq!(next.fragments.len(), 1); + } + + #[test] + fn test_a_later_mint_skips_the_reserved_range() { + let next = apply(&backed_manifest(), vec![reserve(3), add_fragment(0)]).unwrap(); + + assert_eq!( + next.fragments.iter().map(|f| f.id).collect::>(), + vec![0, 4], + "the minted fragment must not land inside the reserved range" + ); + } + + #[test] + fn test_a_reserved_id_is_usable_by_the_next_operation() { + let reserved = apply(&backed_manifest(), vec![reserve(2)]).unwrap(); + assert_eq!(reserved.max_fragment_id(), Some(2)); + + // The next operation mints past the range rather than into it: the + // reservation holds even though nothing was written against it. + let next = apply(&reserved, vec![add_fragment(0)]).unwrap(); + assert_eq!( + next.fragments.iter().map(|f| f.id).collect::>(), + vec![0, 3] + ); + } + + #[test] + fn test_reserving_nothing_is_a_no_op() { + let manifest = backed_manifest(); + let next = apply(&manifest, vec![reserve(0)]).unwrap(); + + assert_eq!(next.max_fragment_id(), manifest.max_fragment_id()); + } +} diff --git a/rust/lance-table/src/transaction/action/reset_table.rs b/rust/lance-table/src/transaction/action/reset_table.rs new file mode 100644 index 00000000000..c39198d7282 --- /dev/null +++ b/rust/lance-table/src/transaction/action/reset_table.rs @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Reset the table to an empty state. + +use super::Footprint; +use super::apply::ApplyState; +use crate::format::pb; +use lance_core::Result; +use lance_core::deepsize::DeepSizeOf; + +/// Reset the table to an empty state, in preparation for a fresh schema and +/// data written by later actions in the same operation. +/// +/// This is how a full `Overwrite` / `CREATE OR REPLACE` decomposes. It drops the +/// entire schema, the schema metadata, all fragments and all indices, and +/// preserves the table config, the table metadata and the base paths -- change +/// those with a [`ConfigUpdate`](super::ConfigUpdate) or an +/// [`AddBase`](super::AddBase) in the same operation. +/// +/// The id counters are not reset. A field or fragment added after the reset gets +/// a fresh id, so a stale file naming an old id can never be mistaken for the +/// new table's data. +#[derive(Debug, Clone, PartialEq, DeepSizeOf, Default)] +pub struct ResetTable; + +impl ResetTable { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + state.reset(); + Ok(()) + } + + /// Emptying the table discards every row it held. + pub(super) fn is_data_change(&self) -> bool { + true + } + + /// Everything. A reset writes every coordinate there is, including ones a + /// concurrent set would only mint, so it takes the table exclusively rather + /// than enumerating them. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.take_exclusive(); + } +} + +impl From<&ResetTable> for pb::ResetTable { + fn from(_value: &ResetTable) -> Self { + Self {} + } +} + +impl TryFrom for ResetTable { + type Error = lance_core::Error; + + fn try_from(_message: pb::ResetTable) -> Result { + Ok(Self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::action::test_support::{ + added_field, apply, apply_with_indices, backed_manifest, + }; + use crate::transaction::action::{Action, AddDataFile, AddField, AddFragment, Ref}; + use crate::transaction::test_support::sample_index_metadata; + use lance_file::version::ConcreteFileVersion; + + fn reset() -> Action { + Action::ResetTable(ResetTable) + } + + #[test] + fn test_reset_empties_the_table() { + let manifest = backed_manifest(); + assert!(!manifest.fragments.is_empty()); + assert!(!manifest.schema.fields.is_empty()); + + let (next, indices) = + apply_with_indices(&manifest, vec![reset()], vec![sample_index_metadata("idx")]) + .unwrap(); + + assert!(next.fragments.is_empty()); + assert!(next.schema.fields.is_empty()); + assert!(indices.is_empty()); + } + + #[test] + fn test_reset_preserves_config_and_base_paths() { + let mut manifest = backed_manifest(); + manifest.config.insert("lance.keep".into(), "yes".into()); + manifest.table_metadata.insert("owner".into(), "me".into()); + + let next = apply(&manifest, vec![reset()]).unwrap(); + + assert_eq!(next.config.get("lance.keep"), Some(&"yes".to_string())); + assert_eq!(next.table_metadata.get("owner"), Some(&"me".to_string())); + } + + #[test] + fn test_reset_then_rebuild_in_one_operation() { + let next = apply( + &backed_manifest(), + vec![ + reset(), + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("fresh"), + }), + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: DataFile::new_unstarted("data/fresh.lance", ConcreteFileVersion::V2_0), + field_ids: vec![Ref::Local(0)], + data_change: true, + }), + ], + ) + .unwrap(); + + // The rebuilt table holds only what the operation wrote after the reset, + // and its ids continue past the ones the old table used. + assert_eq!(next.schema.fields.len(), 1); + let field = next.schema.field("fresh").unwrap(); + assert_ne!(field.id, 0, "a fresh field must not reuse a dropped id"); + assert_eq!(next.fragments.len(), 1); + assert_ne!(next.fragments[0].id, 0); + assert_eq!(next.fragments[0].files[0].fields.as_ref(), &[field.id]); + } + + #[test] + fn test_reset_conflicts_with_everything() { + use crate::transaction::action::{CompositeOperation, Footprint, UserAction}; + + let footprint = |actions| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + }; + + let reset = footprint(vec![reset()]); + // Even a pure append, which writes no committed coordinate at all, is + // preempted: its rows would either vanish or survive the reset + // depending on which commit landed first. + let append = footprint(vec![Action::AddFragment(AddFragment { + local: 0, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + })]); + + assert!(reset.conflicts_with(&append)); + assert!(append.conflicts_with(&reset)); + assert!(reset.conflicts_with(&reset.clone())); + assert!(!append.conflicts_with(&append.clone())); + } +} diff --git a/rust/lance-table/src/transaction/action/set_deletion_file.rs b/rust/lance-table/src/transaction/action/set_deletion_file.rs new file mode 100644 index 00000000000..02eac9b28e3 --- /dev/null +++ b/rust/lance-table/src/transaction/action/set_deletion_file.rs @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Set (replace) a fragment's deletion file. + +use super::apply::ApplyState; +use super::proto::{data_change_from_wire, data_change_to_wire}; +use super::{Coordinate, Footprint}; +use crate::format::{DeletionFile, pb}; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Set (replace) a fragment's deletion file. +/// +/// This is reference-stable rather than a delta: the fragment id is committed +/// and physical row offsets never move, so the post-image is unambiguous. The +/// newly-deleted rows -- the delta rebase and conflict detection need -- are +/// derived by diffing against the read version rather than serialized. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct SetDeletionFile { + /// The fragment, by committed id. + /// + /// Unlike its sibling fragment actions this takes no [`Ref`](super::Ref). + /// A deletion file's path is `{fragment_id}-{read_version}-{id}.{suffix}` + /// (see [`deletion_file_path`](crate::io::deletion::deletion_file_path)), + /// so the writer has to know the committed fragment id before it can write + /// the file at all, and a minted id does not exist until apply. + pub fragment: u64, + /// The new deletion file, or `None` to clear the fragment's deletions. + pub deletion_file: Option, + /// See [`AddFragment::data_change`](super::AddFragment::data_change). + pub data_change: bool, +} + +impl SetDeletionFile { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + state + .fragment_mut(self.fragment, "SetDeletionFile")? + .deletion_file = self.deletion_file.clone(); + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// The fragment's deletions, which is a distinct coordinate from the data of + /// any field in it: deleting rows and re-encoding a column commute. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::FragmentDeletions(self.fragment)); + } +} + +impl From<&SetDeletionFile> for pb::SetDeletionFile { + fn from(value: &SetDeletionFile) -> Self { + Self { + fragment: value.fragment, + deletion_file: value.deletion_file.as_ref().map(pb::DeletionFile::from), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for SetDeletionFile { + type Error = Error; + + fn try_from(message: pb::SetDeletionFile) -> Result { + Ok(Self { + fragment: message.fragment, + deletion_file: message + .deletion_file + .map(DeletionFile::try_from) + .transpose()?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DeletionFileType; + use crate::transaction::action::Action; + use crate::transaction::action::test_support::{apply, backed_manifest}; + + #[test] + fn test_set_deletion_file_sets_and_clears() { + let manifest = backed_manifest(); + let deletion_file = DeletionFile { + read_version: manifest.version, + id: 3, + file_type: DeletionFileType::Array, + num_deleted_rows: Some(2), + base_id: None, + }; + let next = apply( + &manifest, + vec![Action::SetDeletionFile(SetDeletionFile { + fragment: 0, + deletion_file: Some(deletion_file.clone()), + data_change: true, + })], + ) + .unwrap(); + assert_eq!(next.fragments[0].deletion_file, Some(deletion_file)); + + // An absent deletion file is a request to clear it, not a no-op. + let cleared = apply( + &next, + vec![Action::SetDeletionFile(SetDeletionFile { + fragment: 0, + deletion_file: None, + data_change: true, + })], + ) + .unwrap(); + assert_eq!(cleared.fragments[0].deletion_file, None); + } + + #[test] + fn test_set_deletion_file_rejects_a_missing_fragment() { + let error = apply( + &backed_manifest(), + vec![Action::SetDeletionFile(SetDeletionFile { + fragment: 7, + deletion_file: None, + data_change: true, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + } +} diff --git a/rust/lance-table/src/transaction/action/test_support.rs b/rust/lance-table/src/transaction/action/test_support.rs new file mode 100644 index 00000000000..15a92932d2a --- /dev/null +++ b/rust/lance-table/src/transaction/action/test_support.rs @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Fixtures shared by the per-action test modules. + +use super::{Action, CompositeOperation, UserAction}; +use crate::format::{DataFile, Fragment, IndexMetadata, Manifest}; +use crate::transaction::test_support::{default_build_config, sample_manifest}; +use crate::transaction::{Operation, Transaction}; +use arrow_schema::{DataType, Field as ArrowField}; +use lance_core::Result; +use lance_core::datatypes::Field; +use lance_file::version::ConcreteFileVersion; +use std::sync::Arc; + +pub(super) fn apply(manifest: &Manifest, actions: Vec) -> Result { + apply_with_indices(manifest, actions, Vec::new()).map(|(manifest, _)| manifest) +} + +pub(super) fn apply_with_indices( + manifest: &Manifest, + actions: Vec, + indices: Vec, +) -> Result<(Manifest, Vec)> { + let transaction = Transaction::new( + manifest.version, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])), + None, + ); + transaction.build_manifest(Some(manifest), indices, "tx.txn", &default_build_config()) +} + +pub(super) fn added_field(name: &str) -> Field { + Field::try_from(ArrowField::new(name, DataType::Int32, true)).unwrap() +} + +/// `sample_manifest` with fragment 0 actually backed by a data file, so the +/// reference-stable actions have something committed to point at. +pub(super) fn backed_manifest() -> Manifest { + let mut manifest = sample_manifest(); + let mut fragment = Fragment::new(0); + fragment.physical_rows = Some(10); + fragment.files.push(DataFile::new( + "data/0.lance", + vec![0], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + )); + manifest.fragments = Arc::new(vec![fragment]); + manifest +} diff --git a/rust/lance-table/src/transaction/action/tombstone_field_data.rs b/rust/lance-table/src/transaction/action/tombstone_field_data.rs new file mode 100644 index 00000000000..6cecf3ed2df --- /dev/null +++ b/rust/lance-table/src/transaction/action/tombstone_field_data.rs @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Tombstone the data-file binding of committed fields within one fragment. + +use super::apply::{ApplyState, TOMBSTONED_FIELD}; +use super::proto::{data_change_from_wire, data_change_to_wire, required}; +use super::{Footprint, Ref}; +use crate::format::pb; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Tombstone the data-file binding of committed fields within one fragment. +/// +/// Each field's slot in whatever file currently backs it is marked tombstoned, +/// and a file left with no live field is pruned at apply. Data files have no id +/// of their own and a live field is backed by exactly one file, so this is how a +/// column's data is dropped or superseded: re-encoding a column is a tombstone +/// followed by an [`AddDataFile`](super::AddDataFile) for the same field. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct TombstoneFieldData { + pub fragment: Ref, + /// The fields whose current backing is tombstoned. + pub field_ids: Vec, + /// `false` marks a tombstone whose data is re-added by an + /// [`AddDataFile`](super::AddDataFile) for the same fields in the same + /// operation, i.e. a re-encode that moves the bytes without changing any + /// row's value. A tombstone with no matching re-add nulls the column out + /// and is a data change. + pub data_change: bool, +} + +impl TombstoneFieldData { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + let fragment_id = state.resolve_fragment(self.fragment)?; + let field_ids = self + .field_ids + .iter() + .map(|field| state.resolve_field(*field)) + .collect::>>()?; + let fragment = state.fragment_mut(fragment_id, "TombstoneFieldData")?; + + for &field_id in &field_ids { + let mut found = false; + for file in fragment.files.iter_mut() { + let Some(position) = file.fields.iter().position(|id| *id == field_id) else { + continue; + }; + let mut fields = file.fields.to_vec(); + fields[position] = TOMBSTONED_FIELD; + file.fields = fields.into(); + found = true; + } + if !found { + return Err(Error::invalid_input(format!( + "TombstoneFieldData names field {field_id}, which no data file in fragment \ + {fragment_id} backs" + ))); + } + } + + // New values for these fields supersede any overlay still shadowing + // them, so the drop is not silently masked by stale overlay cells. + let overlaid: Vec = field_ids + .iter() + .filter_map(|id| u32::try_from(*id).ok()) + .collect(); + crate::format::overlay::tombstone_overlay_fields(&mut fragment.overlays, &overlaid); + + state.rebind_fields(fragment_id, field_ids.iter().copied()); + Ok(()) + } + + pub(super) fn is_data_change(&self) -> bool { + self.data_change + } + + /// The data of each named field in this fragment, and nothing else: another + /// field's data in the same fragment is untouched. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add_field_data(self.fragment, self.field_ids.iter().copied()); + } +} + +impl From<&TombstoneFieldData> for pb::TombstoneFieldData { + fn from(value: &TombstoneFieldData) -> Self { + Self { + fragment: Some(value.fragment.into()), + field_ids: value.field_ids.iter().map(|id| (*id).into()).collect(), + data_change: data_change_to_wire(value.data_change), + } + } +} + +impl TryFrom for TombstoneFieldData { + type Error = Error; + + fn try_from(message: pb::TombstoneFieldData) -> Result { + Ok(Self { + fragment: required(message.fragment, "TombstoneFieldData.fragment")?.try_into()?, + field_ids: message + .field_ids + .into_iter() + .map(Ref::try_from) + .collect::>>()?, + data_change: data_change_from_wire(message.data_change), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::DataFile; + use crate::transaction::action::Action; + use crate::transaction::action::test_support::{apply, apply_with_indices, backed_manifest}; + use crate::transaction::test_support::sample_index_metadata; + use lance_file::version::ConcreteFileVersion; + use std::sync::Arc; + + fn tombstone_field_zero() -> Action { + Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(0), + field_ids: vec![Ref::Committed(0)], + data_change: true, + }) + } + + #[test] + fn test_tombstone_field_data_drops_the_backing_file() { + let next = apply(&backed_manifest(), vec![tombstone_field_zero()]).unwrap(); + + // The file backed only field 0, so tombstoning it leaves nothing behind. + assert!(next.fragments[0].files.is_empty()); + } + + #[test] + fn test_tombstone_field_data_keeps_a_file_with_a_live_field() { + let mut manifest = backed_manifest(); + let mut fragment = manifest.fragments[0].clone(); + fragment.files[0] = DataFile::new( + "data/0.lance", + vec![0, 1], + vec![0, 1], + ConcreteFileVersion::V2_0, + None, + None, + ); + manifest.fragments = Arc::new(vec![fragment]); + + let next = apply(&manifest, vec![tombstone_field_zero()]).unwrap(); + + assert_eq!( + next.fragments[0].files[0].fields.as_ref(), + &[TOMBSTONED_FIELD, 1] + ); + } + + #[test] + fn test_tombstone_field_data_prunes_the_fragment_from_covering_indices() { + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![tombstone_field_zero()], + vec![sample_index_metadata("idx")], + ) + .unwrap(); + + // The index covers field 0, whose data in fragment 0 is now gone. + assert!(indices[0].fragment_bitmap.as_ref().unwrap().is_empty()); + } + + #[test] + fn test_tombstone_field_data_rejects_a_field_no_file_backs() { + let error = apply( + &backed_manifest(), + vec![Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(0), + field_ids: vec![Ref::Committed(7)], + data_change: true, + })], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("field 7"), "{error}"); + } + + #[test] + fn test_tombstone_field_data_can_name_a_field_minted_in_the_same_operation() { + // A squash of "add column with data" and "re-encode that column" lowers + // to exactly this, and has no committed id to name the field by. + use crate::transaction::action::test_support::added_field; + use crate::transaction::action::{AddDataFile, AddField}; + + let next = apply( + &backed_manifest(), + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: added_field("fresh"), + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(0), + file: DataFile::new_unstarted("data/fresh.lance", ConcreteFileVersion::V1), + field_ids: vec![Ref::Local(0)], + data_change: true, + }), + Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(0), + field_ids: vec![Ref::Local(0)], + data_change: false, + }), + ], + ) + .unwrap(); + + // The file that backed only the minted field is left backing nothing + // and is pruned, so the fragment keeps just its original file. + assert_eq!(next.fragments[0].files.len(), 1); + assert!( + !next.fragments[0] + .files + .iter() + .any(|file| file.path == "data/fresh.lance") + ); + } +} diff --git a/rust/lance-table/src/transaction/action/translate.rs b/rust/lance-table/src/transaction/action/translate.rs new file mode 100644 index 00000000000..fdc2fcbfea4 --- /dev/null +++ b/rust/lance-table/src/transaction/action/translate.rs @@ -0,0 +1,1122 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Translating a legacy [`Operation`] into the action vocabulary. +//! +//! Each named operation is a fixed recipe over the granular actions. Expressing +//! the recipe here rather than in the commit loop is what lets several +//! operations be squashed into one commit: once both are action sets, combining +//! them is concatenation. +//! +//! Translation is fail-closed. An operation whose recipe is not written yet, or +//! one carrying a detail the actions cannot express, is rejected rather than +//! silently translated into something narrower. +//! +//! `Merge` and `Project` are not translated. Both hand over a whole new schema +//! rather than a description of what changed, so recovering the delta needs the +//! read version's schema to diff against -- which this conversion, taking only +//! the operation, does not have. The actions themselves are sufficient: +//! `Project` is a set of [`DropField`](super::DropField)s and `Merge` a set of +//! [`AddField`](super::AddField)s plus their data files. +//! +//! `Update` translates in its vertical form -- rows leaving the fragments they +//! were in and arriving in new ones -- which is a `Delete` and an `Append` in +//! one step, plus the key assertion and SSTable progress it carries. Its other +//! forms are rejected, and for the same reason `Merge` and `Project` are: they +//! turn on what the read version holds. A row rewrite decides which indices +//! still cover the rows it moved by reading the current indices, schema and +//! overlays; a column rewrite tombstones overlaid fields by reading the +//! overlays the fragment carries now; and in-place field modification would +//! have to diff against the read version to tell the data files it wrote from +//! the ones already there. + +use super::{ + Action, AddBase, AddDataFile, AddFragment, AddIndexSegment, AddOverlays, AssertUniqueKeys, Ref, + RemoveFragment, RemoveIndexSegment, SetDeletionFile, TombstoneFieldData, + UpdateCompactedSsTables, UserAction, +}; +use crate::format::key_existence::KeyExistenceFilter; +use crate::format::{Fragment, IndexMetadata}; +use crate::system_index::mem_wal::CompactedSsTable; +use crate::transaction::{DataReplacementGroup, Operation, UpdateMode, UpdatedFragmentOffsets}; +use lance_core::{Error, Result}; + +impl TryFrom<&Operation> for Vec { + type Error = Error; + + fn try_from(operation: &Operation) -> Result { + match operation { + Operation::Append { fragments } => Ok(vec![UserAction::new( + format!("append {} fragments", fragments.len()), + append_actions(fragments)?, + )]), + Operation::Delete { + updated_fragments, + deleted_fragment_ids, + predicate, + } => Ok(vec![UserAction::new( + format!("delete rows matching {predicate}"), + delete_actions(updated_fragments, deleted_fragment_ids), + )]), + Operation::UpdateBases { new_bases } => Ok(vec![UserAction::new( + format!("add {} base paths", new_bases.len()), + new_bases + .iter() + .enumerate() + .map(|(index, base)| { + Action::AddBase(AddBase { + local: index as u32, + base: base.clone(), + }) + }) + .collect(), + )]), + Operation::CreateIndex { + new_indices, + removed_indices, + } => Ok(vec![UserAction::new( + describe_index_change(new_indices, removed_indices), + create_index_actions(new_indices, removed_indices)?, + )]), + Operation::DataOverlay { groups } => Ok(vec![UserAction::new( + format!("overlay {} fragments", groups.len()), + groups + .iter() + .map(|group| { + Action::AddOverlays(AddOverlays { + fragment: Ref::Committed(group.fragment_id), + overlays: group.overlays.clone(), + data_change: true, + }) + }) + .collect(), + )]), + // An empty list is a no-op the legacy path tolerates, and an + // UpdateCompactedSsTables naming no SSTable is rejected, so it + // translates to a step with nothing in it rather than an action. + // Requiring index catch-up is a one-way feature-flag migration with + // no action of its own, so an operation asking for it does not + // translate. + Operation::UpdateMemWalState { + require_index_catchup: true, + .. + } => Err(Error::not_supported( + "translating a MemWAL state update that requires index catch-up into actions", + )), + Operation::UpdateMemWalState { + compacted_sstables, .. + } => Ok(vec![UserAction::new( + format!("compact {} MemWAL SSTables", compacted_sstables.len()), + if compacted_sstables.is_empty() { + Vec::new() + } else { + vec![Action::UpdateCompactedSsTables(UpdateCompactedSsTables { + compacted_sstables: compacted_sstables.clone(), + })] + }, + )]), + Operation::Update { + removed_fragment_ids, + updated_fragments, + new_fragments, + fields_modified, + compacted_sstables, + update_mode, + inserted_rows_filter, + updated_fragment_offsets, + // Read only by the index coverage preservation that + // `RewriteRows` asks for, which is rejected below. + fields_for_preserving_frag_bitmap: _, + } => { + reject_untranslatable_update( + update_mode, + fields_modified, + updated_fragment_offsets, + )?; + Ok(vec![UserAction::new( + describe_update(updated_fragments, new_fragments, removed_fragment_ids), + update_actions( + updated_fragments, + removed_fragment_ids, + new_fragments, + compacted_sstables, + inserted_rows_filter.as_ref(), + )?, + )]) + } + Operation::DataReplacement { replacements } => Ok(vec![UserAction::new( + format!("replace data files in {} fragments", replacements.len()), + data_replacement_actions(replacements)?, + )]), + other => Err(Error::not_supported(format!( + "translating a {} operation into actions", + other.name() + ))), + } + } +} + +fn append_actions(fragments: &[Fragment]) -> Result> { + let mut actions = Vec::with_capacity(fragments.len() * 2); + for (index, fragment) in fragments.iter().enumerate() { + let local = index as u32; + let physical_rows = fragment.physical_rows.ok_or_else(|| { + Error::invalid_input( + "an appended fragment must know its physical row count to become an AddFragment", + ) + })?; + // A fragment being appended has no committed state, so anything that + // only makes sense against committed data means the caller built the + // operation by hand out of an existing fragment. + if fragment.deletion_file.is_some() || !fragment.overlays.is_empty() { + return Err(Error::invalid_input(format!( + "appended fragment {} carries a deletion file or overlays, which an append \ + cannot produce", + fragment.id + ))); + } + + actions.push(Action::AddFragment(AddFragment { + local, + physical_rows: physical_rows as u64, + row_id_meta: fragment.row_id_meta.clone(), + last_updated_at_version_meta: fragment.last_updated_at_version_meta.clone(), + created_at_version_meta: fragment.created_at_version_meta.clone(), + data_change: true, + })); + + for file in &fragment.files { + actions.push(Action::AddDataFile(AddDataFile { + fragment: Ref::Local(local), + file: file.clone(), + field_ids: committed_field_refs(file.fields.as_ref())?, + data_change: true, + })); + } + } + Ok(actions) +} + +/// A legacy delete replaces whole fragments, but the only thing it ever changes +/// on one is its deletion file, so that is what the translation carries over. +fn delete_actions(updated_fragments: &[Fragment], deleted_fragment_ids: &[u64]) -> Vec { + let mut actions = Vec::with_capacity(updated_fragments.len() + deleted_fragment_ids.len()); + for fragment in updated_fragments { + actions.push(Action::SetDeletionFile(SetDeletionFile { + fragment: fragment.id, + deletion_file: fragment.deletion_file.clone(), + data_change: true, + })); + } + for id in deleted_fragment_ids { + actions.push(Action::RemoveFragment(RemoveFragment { + fragment: Ref::Committed(*id), + data_change: true, + })); + } + actions +} + +/// The parts of an `Update` that cannot be recovered from the operation alone. +/// +/// Each of these is a decision the legacy path makes by reading the version +/// being committed against. Rejecting them keeps the translation honest: a +/// silent omission would not fail the commit, it would leave an index quietly +/// uncovered or a row's version stamp quietly stale. +fn reject_untranslatable_update( + update_mode: &Option, + fields_modified: &[u32], + updated_fragment_offsets: &Option, +) -> Result<()> { + match update_mode { + // A pure row rewrite lets an index keep covering rows that moved into a + // new fragment, but only when the rewrite provably left the indexed + // columns alone. Deciding that reads the read version's indices, its + // schema, and the overlays the original fragments carried. + Some(UpdateMode::RewriteRows) => Err(Error::not_supported( + "translating a row-rewriting update into actions, which would have to decide from \ + the read version which indices still cover the rewritten rows", + )), + // An in-place column rewrite tombstones the overlaid fields its fresh + // base values supersede, which needs the overlays the fragment carries + // in the read version rather than the ones the operation restates. + Some(UpdateMode::RewriteColumns) => Err(Error::not_supported( + "translating a column-rewriting update into actions, which would have to read the \ + overlays the read version's fragments carry", + )), + None if !fields_modified.is_empty() => Err(Error::not_supported( + "translating an update that modifies fields in place into actions, which would have \ + to diff against the read version to tell the data files it wrote from the ones \ + already there", + )), + None if updated_fragment_offsets + .as_ref() + .is_some_and(|offsets| !offsets.0.is_empty()) => + { + Err(Error::not_supported( + "translating an update that restamps part of a fragment into actions; \ + RefreshRowVersionMetadata restamps every row of the fragments it names", + )) + } + None => Ok(()), + } +} + +fn describe_update( + updated_fragments: &[Fragment], + new_fragments: &[Fragment], + removed_fragment_ids: &[u64], +) -> String { + format!( + "update {} fragments, adding {} and removing {}", + updated_fragments.len(), + new_fragments.len(), + removed_fragment_ids.len() + ) +} + +/// What an update does to the fragment list, plus the precondition and the +/// bookkeeping it carries. +/// +/// The structural part is a delete and an append: rows leave the fragments they +/// were in, by deletion file or by the fragment going away entirely, and arrive +/// in fragments the operation mints. Those are the same changes `Delete` and +/// `Append` describe, so this reuses their recipes rather than restating them. +/// +/// The assertion goes first, so a commit rejected for a key collision is +/// rejected before anything else is read. +fn update_actions( + updated_fragments: &[Fragment], + removed_fragment_ids: &[u64], + new_fragments: &[Fragment], + compacted_sstables: &[CompactedSsTable], + inserted_rows_filter: Option<&KeyExistenceFilter>, +) -> Result> { + let mut actions = Vec::with_capacity( + updated_fragments.len() + removed_fragment_ids.len() + new_fragments.len() * 2 + 2, + ); + + if let Some(filter) = inserted_rows_filter { + if filter.field_ids.is_empty() { + return Err(Error::invalid_input( + "an update carries a filter of inserted keys that names no key column, so \ + nothing says which keys the filter is over", + )); + } + actions.push(Action::AssertUniqueKeys(AssertUniqueKeys { + key_fields: committed_field_refs(&filter.field_ids)?, + filter: filter.clone(), + })); + } + + actions.extend(delete_actions(updated_fragments, removed_fragment_ids)); + actions.extend(append_actions(new_fragments)?); + + if !compacted_sstables.is_empty() { + actions.push(Action::UpdateCompactedSsTables(UpdateCompactedSsTables { + compacted_sstables: compacted_sstables.to_vec(), + })); + } + + Ok(actions) +} + +/// Replacing a field's data is a drop of the old backing file followed by an +/// add of the new one. The legacy path swaps the path on the existing file in +/// place instead, so the resulting fragment holds the same set of data files +/// but not necessarily in the same order -- files are addressed by field, so +/// the order carries no meaning. +fn data_replacement_actions(replacements: &[DataReplacementGroup]) -> Result> { + let mut actions = Vec::with_capacity(replacements.len() * 2); + for DataReplacementGroup(fragment_id, new_file) in replacements { + let fragment = Ref::Committed(*fragment_id); + actions.push(Action::TombstoneFieldData(TombstoneFieldData { + fragment, + field_ids: committed_field_refs(new_file.fields.as_ref())?, + data_change: true, + })); + actions.push(Action::AddDataFile(AddDataFile { + fragment, + file: new_file.clone(), + field_ids: committed_field_refs(new_file.fields.as_ref())?, + data_change: true, + })); + } + Ok(actions) +} + +fn describe_index_change(new: &[IndexMetadata], removed: &[IndexMetadata]) -> String { + match (new.is_empty(), removed.is_empty()) { + (false, true) => format!("build {} index segments", new.len()), + (true, false) => format!("drop {} index segments", removed.len()), + _ => format!( + "replace {} index segments with {}", + removed.len(), + new.len() + ), + } +} + +/// A legacy index change is a set of segment removals followed by a set of +/// additions -- which is what it already was, since the operation carries the +/// two lists separately. +/// +/// Two edges the legacy path tolerates and this does not. It drops a named +/// removal that is not in the manifest, and it drops an existing segment whose +/// uuid a new one reuses; both are rejected here, because either one means the +/// operation was planned against a different set of segments than it is landing +/// on. +fn create_index_actions( + new_indices: &[IndexMetadata], + removed_indices: &[IndexMetadata], +) -> Result> { + let mut actions = Vec::with_capacity(new_indices.len() + removed_indices.len()); + for index in removed_indices { + actions.push(Action::RemoveIndexSegment(RemoveIndexSegment { + uuid: index.uuid, + data_change: false, + })); + } + for index in new_indices { + actions.push(Action::AddIndexSegment(AddIndexSegment { + uuid: index.uuid, + name: index.name.clone(), + fields: committed_field_refs(&index.fields)?, + index_details: index.index_details.clone(), + index_version: index.index_version, + covered_fragments: index.fragment_bitmap.as_ref().map(|bitmap| { + bitmap + .iter() + .map(|fragment| Ref::Committed(u64::from(fragment))) + .collect() + }), + files: index.files.clone().unwrap_or_default(), + base: index.base_id.map(|id| Ref::Committed(u64::from(id))), + created_at: index.created_at, + dataset_version: Some(index.dataset_version), + // The legacy operation carries no such marker, and an index is + // derived from data this operation does not touch. + data_change: false, + })); + } + Ok(actions) +} + +fn committed_field_refs(field_ids: &[i32]) -> Result> { + field_ids + .iter() + .map(|id| { + u64::try_from(*id).map(Ref::Committed).map_err(|_| { + Error::invalid_input(format!( + "a data file in this operation lists field id {id}, which is not a committed \ + field" + )) + }) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::format::key_existence::KeyExistenceFilterBuilder; + use crate::format::{ + BasePath, DataFile, DeletionFile, DeletionFileType, IndexMetadata, Manifest, RowIdMeta, + }; + use crate::rowids::{RowIdSequence, write_row_ids}; + use crate::system_index::mem_wal::{ + CompactedSsTable, MEM_WAL_INDEX_NAME, MemWalIndexDetails, load_mem_wal_index_details, + new_mem_wal_index_meta, + }; + use crate::transaction::DataOverlayGroup; + use crate::transaction::Transaction; + use crate::transaction::action::CompositeOperation; + use crate::transaction::test_support::{ + default_build_config, make_stable_row_id_manifest, overlay_with_field, + sample_index_metadata, sample_manifest, + }; + use lance_file::version::ConcreteFileVersion; + use roaring::RoaringBitmap; + use rstest::rstest; + use std::sync::Arc; + use uuid::Uuid; + + /// Build the same manifest twice -- once down the legacy path, once by + /// translating the operation to actions -- and assert they agree. + fn assert_parity(manifest: &Manifest, operation: Operation) -> Manifest { + assert_parity_with_indices(manifest, operation, Vec::new()).0 + } + + fn assert_parity_with_indices( + manifest: &Manifest, + operation: Operation, + indices: Vec, + ) -> (Manifest, Vec) { + let (legacy, legacy_indices) = build(manifest, operation.clone(), indices.clone()); + + let actions = Vec::::try_from(&operation).unwrap(); + let (translated, translated_indices) = build( + manifest, + Operation::CompositeOperation(CompositeOperation::new(actions)), + indices, + ); + + // Data files are addressed by field, so the two paths are allowed to + // hold them in different orders within a fragment. + assert_eq!(translated.fragments.len(), legacy.fragments.len()); + for (translated, legacy) in translated.fragments.iter().zip(legacy.fragments.iter()) { + assert_eq!(sorted_files(translated), sorted_files(legacy)); + assert_eq!( + Fragment { + files: Vec::new(), + ..translated.clone() + }, + Fragment { + files: Vec::new(), + ..legacy.clone() + } + ); + } + assert_eq!(translated.schema, legacy.schema); + assert_eq!(translated.base_paths, legacy.base_paths); + assert_eq!(translated.next_row_id, legacy.next_row_id); + assert_eq!(translated.max_fragment_id, legacy.max_fragment_id); + assert_eq!(translated_indices, legacy_indices); + (translated, translated_indices) + } + + fn sorted_files(fragment: &Fragment) -> Vec { + let mut files = fragment.files.clone(); + files.sort_by(|a, b| a.path.cmp(&b.path)); + files + } + + fn build( + manifest: &Manifest, + operation: Operation, + indices: Vec, + ) -> (Manifest, Vec) { + Transaction::new(manifest.version, operation, None) + .build_manifest(Some(manifest), indices, "tx.txn", &default_build_config()) + .unwrap() + } + + fn appendable_fragment(path: &str) -> Fragment { + let mut fragment = Fragment::new(0); + fragment.physical_rows = Some(10); + fragment.files.push(DataFile::new( + path, + vec![0], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + )); + fragment + } + + fn manifest_with_fragments(fragments: Vec) -> Manifest { + let mut manifest = sample_manifest(); + manifest.fragments = Arc::new(fragments); + manifest + } + + #[test] + fn test_append_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let next = assert_parity( + &manifest, + Operation::Append { + fragments: vec![ + appendable_fragment("data/1.lance"), + appendable_fragment("data/2.lance"), + ], + }, + ); + + // The appended fragments take ids from the manifest's counter, not the + // zero they were handed to the operation with. + assert_eq!( + next.fragments.iter().map(|f| f.id).collect::>(), + vec![0, 1, 2] + ); + } + + #[test] + fn test_append_assigns_stable_row_ids_like_the_legacy_path() { + let mut existing = appendable_fragment("data/1.lance"); + existing.id = 1; + existing.row_id_meta = Some(RowIdMeta::Inline( + write_row_ids(&RowIdSequence::from(0u64..10)).into(), + )); + let manifest = make_stable_row_id_manifest(vec![existing]); + + let next = assert_parity( + &manifest, + Operation::Append { + fragments: vec![appendable_fragment("data/2.lance")], + }, + ); + + assert_eq!(next.next_row_id, 1010); + let appended = next.fragments.iter().find(|f| f.id == 2).unwrap(); + assert!(appended.row_id_meta.is_some()); + assert!(appended.created_at_version_meta.is_some()); + } + + #[test] + fn test_append_rejects_a_fragment_without_a_row_count() { + let operation = Operation::Append { + fragments: vec![Fragment::new(0)], + }; + let error = Vec::::try_from(&operation).unwrap_err(); + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + } + + #[test] + fn test_delete_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance"), { + let mut fragment = appendable_fragment("data/1.lance"); + fragment.id = 1; + fragment + }]); + + let mut updated = manifest.fragments[0].clone(); + updated.deletion_file = Some(DeletionFile { + read_version: manifest.version, + id: 7, + file_type: DeletionFileType::Array, + num_deleted_rows: Some(3), + base_id: None, + }); + + let next = assert_parity( + &manifest, + Operation::Delete { + updated_fragments: vec![updated], + deleted_fragment_ids: vec![1], + predicate: "id > 5".into(), + }, + ); + + assert_eq!(next.fragments.len(), 1); + assert!(next.fragments[0].deletion_file.is_some()); + } + + #[test] + fn test_update_bases_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let next = assert_parity( + &manifest, + Operation::UpdateBases { + new_bases: vec![ + BasePath::new(0, "s3://bucket/a".into(), Some("a".into()), false), + BasePath::new(0, "s3://bucket/b".into(), Some("b".into()), false), + ], + }, + ); + + assert_eq!(next.base_paths.len(), 2); + } + + #[test] + fn test_data_replacement_matches_the_legacy_path() { + let mut fragment = appendable_fragment("data/0.lance"); + fragment.files.push(DataFile::new( + "data/0b.lance", + vec![1], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + )); + let manifest = manifest_with_fragments(vec![fragment]); + + let replacement = DataFile::new( + "data/0-new.lance", + vec![0], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + ); + let next = assert_parity( + &manifest, + Operation::DataReplacement { + replacements: vec![DataReplacementGroup(0, replacement)], + }, + ); + + let paths = sorted_files(&next.fragments[0]) + .iter() + .map(|file| file.path.clone()) + .collect::>(); + assert_eq!(paths, vec!["data/0-new.lance", "data/0b.lance"]); + } + + #[test] + fn test_data_replacement_of_an_unbacked_field_is_rejected() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let operation = Operation::DataReplacement { + replacements: vec![DataReplacementGroup( + 0, + DataFile::new( + "data/0-new.lance", + vec![9], + vec![0], + ConcreteFileVersion::V2_0, + None, + None, + ), + )], + }; + let actions = Vec::::try_from(&operation).unwrap(); + let error = Transaction::new( + manifest.version, + Operation::CompositeOperation(CompositeOperation::new(actions)), + None, + ) + .build_manifest( + Some(&manifest), + Vec::new(), + "tx.txn", + &default_build_config(), + ) + .unwrap_err(); + + // The legacy path treats this as an all-NULL column gaining real data; + // the action form has no way to say "drop this if it is there". + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + } + + #[test] + fn test_create_index_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let existing = sample_index_metadata("by_b"); + let built = sample_index_metadata("by_a"); + + let (_, indices) = assert_parity_with_indices( + &manifest, + Operation::CreateIndex { + new_indices: vec![built.clone()], + removed_indices: Vec::new(), + }, + vec![existing.clone()], + ); + + assert_eq!(indices.len(), 2); + assert!(indices.contains(&built)); + assert!(indices.contains(&existing)); + } + + #[test] + fn test_dropping_an_index_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let dropped = sample_index_metadata("by_a"); + let kept = sample_index_metadata("by_b"); + + let (_, indices) = assert_parity_with_indices( + &manifest, + Operation::CreateIndex { + new_indices: Vec::new(), + removed_indices: vec![dropped.clone()], + }, + vec![dropped, kept.clone()], + ); + + assert_eq!(indices, vec![kept]); + } + + #[test] + fn test_replacing_an_index_segment_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let old = sample_index_metadata("by_a"); + let new = sample_index_metadata("by_a"); + + let (_, indices) = assert_parity_with_indices( + &manifest, + Operation::CreateIndex { + new_indices: vec![new.clone()], + removed_indices: vec![old.clone()], + }, + vec![old], + ); + + assert_eq!(indices, vec![new]); + } + + #[test] + fn test_removing_an_index_segment_the_dataset_does_not_have_is_rejected() { + // The legacy path silently drops such a removal; the action form says + // the operation was planned against a different set of segments. + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let operation = Operation::CreateIndex { + new_indices: Vec::new(), + removed_indices: vec![sample_index_metadata("by_a")], + }; + let actions = Vec::::try_from(&operation).unwrap(); + let error = Transaction::new( + manifest.version, + Operation::CompositeOperation(CompositeOperation::new(actions)), + None, + ) + .build_manifest( + Some(&manifest), + Vec::new(), + "tx.txn", + &default_build_config(), + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + } + + #[test] + fn test_an_untranslated_operation_is_rejected() { + let operation = Operation::ReserveFragments { num_fragments: 3 }; + let error = Vec::::try_from(&operation).unwrap_err(); + assert!(matches!(error, Error::NotSupported { .. }), "{error:?}"); + assert!(error.to_string().contains("ReserveFragments"), "{error}"); + } + + #[test] + fn test_data_overlay_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance"), { + let mut fragment = appendable_fragment("data/1.lance"); + fragment.id = 1; + fragment + }]); + + let next = assert_parity( + &manifest, + Operation::DataOverlay { + groups: vec![ + DataOverlayGroup { + fragment_id: 0, + overlays: vec![overlay_with_field(0, 0)], + }, + DataOverlayGroup { + fragment_id: 1, + overlays: vec![overlay_with_field(0, 0)], + }, + ], + }, + ); + + // Both paths stamp the version the commit produces over whatever the + // writer left in `committed_version`. + for fragment in next.fragments.iter() { + assert_eq!(fragment.overlays.len(), 1); + assert_eq!(fragment.overlays[0].committed_version, manifest.version + 1); + } + } + + #[test] + fn test_data_overlay_groups_for_one_fragment_are_appended_in_order() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + + let group = |field| DataOverlayGroup { + fragment_id: 0, + overlays: vec![overlay_with_field(field, 0)], + }; + let next = assert_parity( + &manifest, + Operation::DataOverlay { + groups: vec![group(0), group(1)], + }, + ); + + let fields = next.fragments[0] + .overlays + .iter() + .map(|overlay| overlay.data_file.fields.to_vec()) + .collect::>(); + assert_eq!(fields, vec![vec![0], vec![1]]); + } + + #[test] + fn test_overlaying_a_fragment_that_is_not_there_is_rejected() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let operation = Operation::DataOverlay { + groups: vec![DataOverlayGroup { + fragment_id: 7, + overlays: vec![overlay_with_field(0, 0)], + }], + }; + + let actions = Vec::::try_from(&operation).unwrap(); + let error = Transaction::new( + manifest.version, + Operation::CompositeOperation(CompositeOperation::new(actions)), + None, + ) + .build_manifest( + Some(&manifest), + Vec::new(), + "tx.txn", + &default_build_config(), + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + } + + /// The legacy `UpdateMemWalState` arm never carries the read version's + /// fragments into the manifest it builds, so it empties the table. The + /// translated path leaves the data alone, which is what the operation + /// means, so the two agree on the indices but not on the fragments. + #[test] + fn test_update_mem_wal_state_records_the_same_progress_as_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let operation = Operation::UpdateMemWalState { + require_index_catchup: false, + compacted_sstables: vec![CompactedSsTable::new(Uuid::from_u128(1), 4)], + }; + + // Recording progress requires the table to already carry the index. + let existing = + vec![new_mem_wal_index_meta(manifest.version, MemWalIndexDetails::default()).unwrap()]; + let (legacy, legacy_indices) = build(&manifest, operation.clone(), existing.clone()); + let actions = Vec::::try_from(&operation).unwrap(); + let (translated, translated_indices) = build( + &manifest, + Operation::CompositeOperation(CompositeOperation::new(actions)), + existing, + ); + + let progress = |indices: &[IndexMetadata]| { + let mem_wal = indices + .iter() + .find(|index| index.name == MEM_WAL_INDEX_NAME) + .expect("the MemWAL index should be there"); + load_mem_wal_index_details(mem_wal.clone()) + .unwrap() + .compacted_sstables + }; + assert_eq!(progress(&translated_indices), progress(&legacy_indices)); + assert_eq!(translated.fragments, legacy.fragments); + } + + #[test] + fn test_update_mem_wal_state_with_no_sstables_translates_to_no_actions() { + let operation = Operation::UpdateMemWalState { + require_index_catchup: false, + compacted_sstables: Vec::new(), + }; + let actions = Vec::::try_from(&operation).unwrap(); + + assert_eq!(actions.len(), 1); + assert!(actions[0].actions.is_empty()); + } + /// The parts of an `Update` these tests vary, so each one can name only + /// what it is about. Defaults to an update that does nothing. + #[derive(Default)] + struct Update { + updated_fragments: Vec, + removed_fragment_ids: Vec, + new_fragments: Vec, + fields_modified: Vec, + compacted_sstables: Vec, + update_mode: Option, + inserted_rows_filter: Option, + updated_fragment_offsets: Option, + } + + impl From for Operation { + fn from(update: Update) -> Self { + Self::Update { + removed_fragment_ids: update.removed_fragment_ids, + updated_fragments: update.updated_fragments, + new_fragments: update.new_fragments, + fields_modified: update.fields_modified, + compacted_sstables: update.compacted_sstables, + update_mode: update.update_mode, + inserted_rows_filter: update.inserted_rows_filter, + updated_fragment_offsets: update.updated_fragment_offsets, + // Only read by the coverage preservation a row rewrite asks + // for, and no test here gets far enough to use it. + fields_for_preserving_frag_bitmap: vec![], + } + } + } + + fn deleted(mut fragment: Fragment, read_version: u64) -> Fragment { + fragment.deletion_file = Some(DeletionFile { + read_version, + id: 7, + file_type: DeletionFileType::Array, + num_deleted_rows: Some(3), + base_id: None, + }); + fragment + } + + #[test] + fn test_update_matches_the_legacy_path() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance"), { + let mut fragment = appendable_fragment("data/1.lance"); + fragment.id = 1; + fragment + }]); + + let next = assert_parity( + &manifest, + Update { + updated_fragments: vec![deleted(manifest.fragments[0].clone(), manifest.version)], + removed_fragment_ids: vec![1], + new_fragments: vec![appendable_fragment("data/2.lance")], + ..Default::default() + } + .into(), + ); + + // The fragment that kept some rows, and the one the update wrote them + // into. Fragment 1 gave up all of its rows and is gone. + assert_eq!(next.fragments.len(), 2); + assert!(next.fragments[0].deletion_file.is_some()); + assert_eq!(next.fragments[1].files[0].path, "data/2.lance"); + } + + #[test] + fn test_an_update_that_only_deletes_matches_the_legacy_path() { + // The shape merge insert commits when the source only matches rows to + // delete: fragments lose rows, and nothing is written. + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let next = assert_parity( + &manifest, + Update { + updated_fragments: vec![deleted(manifest.fragments[0].clone(), manifest.version)], + ..Default::default() + } + .into(), + ); + + assert!(next.fragments[0].deletion_file.is_some()); + } + + #[test] + fn test_an_update_carries_its_inserted_keys_into_an_assertion() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let filter = + KeyExistenceFilter::from_bloom_filter(&KeyExistenceFilterBuilder::new(vec![0])); + let operation: Operation = Update { + inserted_rows_filter: Some(filter.clone()), + new_fragments: vec![appendable_fragment("data/1.lance")], + ..Default::default() + } + .into(); + + let actions = Vec::::try_from(&operation).unwrap(); + let assertion = actions[0] + .actions + .iter() + .find_map(|action| match action { + Action::AssertUniqueKeys(assertion) => Some(assertion), + _ => None, + }) + .expect("the filter should have become an assertion"); + + assert_eq!(assertion.key_fields, vec![Ref::Committed(0)]); + assert_eq!(assertion.filter, filter); + // The assertion is a precondition, so it changes nothing about the + // manifest either path builds. + assert_parity(&manifest, operation); + } + + #[test] + fn test_an_update_whose_key_filter_names_no_column_is_rejected() { + let operation: Operation = Update { + inserted_rows_filter: Some(KeyExistenceFilter::from_bloom_filter( + &KeyExistenceFilterBuilder::new(vec![]), + )), + ..Default::default() + } + .into(); + + let error = Vec::::try_from(&operation).unwrap_err(); + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!(error.to_string().contains("names no key column"), "{error}"); + } + + #[test] + fn test_an_update_records_mem_wal_compaction_progress() { + let manifest = manifest_with_fragments(vec![appendable_fragment("data/0.lance")]); + let operation: Operation = Update { + compacted_sstables: vec![CompactedSsTable::new(Uuid::from_u128(1), 4)], + updated_fragments: vec![deleted(manifest.fragments[0].clone(), manifest.version)], + ..Default::default() + } + .into(); + + // Both paths re-mint the MemWAL index with a fresh uuid and timestamp, + // so the indices are compared by the progress they record. + let existing = + vec![new_mem_wal_index_meta(manifest.version, MemWalIndexDetails::default()).unwrap()]; + let (legacy, legacy_indices) = build(&manifest, operation.clone(), existing.clone()); + let actions = Vec::::try_from(&operation).unwrap(); + let (translated, translated_indices) = build( + &manifest, + Operation::CompositeOperation(CompositeOperation::new(actions)), + existing, + ); + + let progress = |indices: &[IndexMetadata]| { + let mem_wal = indices + .iter() + .find(|index| index.name == MEM_WAL_INDEX_NAME) + .expect("the MemWAL index should be there"); + load_mem_wal_index_details(mem_wal.clone()) + .unwrap() + .compacted_sstables + }; + assert_eq!(progress(&translated_indices).len(), 1); + assert_eq!(progress(&translated_indices), progress(&legacy_indices)); + assert_eq!(translated.fragments, legacy.fragments); + } + + /// Every form of `Update` whose recipe depends on the version being + /// committed against, which this conversion does not have. + #[rstest] + #[case::rewriting_rows( + Update { update_mode: Some(UpdateMode::RewriteRows), ..Default::default() }, + "which indices still cover the rewritten rows" + )] + #[case::rewriting_columns( + Update { update_mode: Some(UpdateMode::RewriteColumns), ..Default::default() }, + "the overlays the read version's fragments carry" + )] + #[case::modifying_fields_in_place( + Update { fields_modified: vec![0], ..Default::default() }, + "tell the data files it wrote from the ones already there" + )] + #[case::restamping_part_of_a_fragment( + Update { + updated_fragment_offsets: Some(UpdatedFragmentOffsets( + [(0, RoaringBitmap::from_iter([0_u32, 1]))].into_iter().collect(), + )), + ..Default::default() + }, + "restamps every row of the fragments it names" + )] + fn test_an_update_the_operation_does_not_fully_describe_is_rejected( + #[case] update: Update, + #[case] expected: &str, + ) { + let operation = Operation::from(update); + let error = Vec::::try_from(&operation).unwrap_err(); + assert!(matches!(error, Error::NotSupported { .. }), "{error:?}"); + assert!(error.to_string().contains(expected), "{error}"); + } + + /// Offsets nobody filled in are not a partial restamp. + #[test] + fn test_an_update_carrying_empty_offsets_still_translates() { + let operation: Operation = Update { + updated_fragment_offsets: Some(UpdatedFragmentOffsets(Default::default())), + ..Default::default() + } + .into(); + + assert!(Vec::::try_from(&operation).is_ok()); + } +} diff --git a/rust/lance-table/src/transaction/action/update_compacted_sstables.rs b/rust/lance-table/src/transaction/action/update_compacted_sstables.rs new file mode 100644 index 00000000000..ef2838bec3c --- /dev/null +++ b/rust/lance-table/src/transaction/action/update_compacted_sstables.rs @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright The Lance Authors + +//! Record which MemWAL SSTables have been compacted into the base table. + +use super::apply::ApplyState; +use super::{Coordinate, Footprint}; +use crate::format::pb; +use crate::system_index::mem_wal::{CompactedSsTable, MEM_WAL_INDEX_NAME}; +use lance_core::deepsize::DeepSizeOf; +use lance_core::{Error, Result}; + +/// Mark MemWAL SSTables as compacted into the base table. +/// +/// The rows were already readable through the WAL, so this records where they +/// are rather than changing them. A shard's generation may only move forward, +/// and the table must already carry a MemWAL index: progress against a shard +/// nothing corroborates is rejected rather than invented. +/// +/// This is the one action that edits the MemWAL system index rather than the +/// data, which is why it exists at all: the index is a segment like any other, +/// but its contents are compaction bookkeeping that no +/// [`AddIndexSegment`](super::AddIndexSegment) could express as a delta. +#[derive(Debug, Clone, PartialEq, DeepSizeOf)] +pub struct UpdateCompactedSsTables { + pub compacted_sstables: Vec, +} + +impl UpdateCompactedSsTables { + pub(super) fn apply(&self, state: &mut ApplyState) -> Result<()> { + if self.compacted_sstables.is_empty() { + return Err(Error::invalid_input( + "UpdateCompactedSsTables names no SSTable, so there is no progress to record", + )); + } + state.update_compacted_sstables(self.compacted_sstables.clone()) + } + + pub(super) fn is_data_change(&self) -> bool { + false + } + + /// The MemWAL index, by name. Two writers recording compaction progress + /// both rewrite the whole index entry, so the later one would drop what the + /// earlier one recorded. + pub(super) fn footprint(&self, footprint: &mut Footprint) { + footprint.add(Coordinate::IndexName(MEM_WAL_INDEX_NAME.to_string())); + } +} + +impl From<&UpdateCompactedSsTables> for pb::UpdateCompactedSsTables { + fn from(value: &UpdateCompactedSsTables) -> Self { + Self { + compacted_sstables: value + .compacted_sstables + .iter() + .map(pb::CompactedSsTable::from) + .collect(), + } + } +} + +impl TryFrom for UpdateCompactedSsTables { + type Error = Error; + + fn try_from(message: pb::UpdateCompactedSsTables) -> Result { + Ok(Self { + compacted_sstables: message + .compacted_sstables + .into_iter() + .map(CompactedSsTable::try_from) + .collect::>>()?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::system_index::mem_wal::{ + MemWalIndexDetails, load_mem_wal_index_details, new_mem_wal_index_meta, + }; + use crate::transaction::action::test_support::{apply_with_indices, backed_manifest}; + use crate::transaction::action::{Action, CompositeOperation, UserAction}; + use crate::transaction::test_support::sample_index_metadata; + use uuid::Uuid; + + fn update(sstables: Vec<(u128, u64)>) -> Action { + Action::UpdateCompactedSsTables(UpdateCompactedSsTables { + compacted_sstables: sstables + .into_iter() + .map(|(shard, generation)| { + CompactedSsTable::new(Uuid::from_u128(shard), generation) + }) + .collect(), + }) + } + + /// The compaction progress the MemWAL index records, as (shard, generation) + /// pairs sorted by shard. + fn progress(indices: &[crate::format::IndexMetadata]) -> Vec<(u128, u64)> { + let mem_wal = indices + .iter() + .find(|index| index.name == MEM_WAL_INDEX_NAME) + .expect("the MemWAL index should be there"); + let details = load_mem_wal_index_details(mem_wal.clone()).unwrap(); + let mut progress = details + .compacted_sstables + .iter() + .map(|sstable| (sstable.shard_id.as_u128(), sstable.generation)) + .collect::>(); + progress.sort(); + progress + } + + /// A MemWAL index recording no compaction progress yet, which recording + /// progress requires the table to already have. + fn empty_mem_wal_index() -> crate::format::IndexMetadata { + new_mem_wal_index_meta(1, MemWalIndexDetails::default()).unwrap() + } + + #[test] + fn test_recording_progress_without_a_mem_wal_index_is_rejected() { + let error = apply_with_indices(&backed_manifest(), vec![update(vec![(1, 7)])], Vec::new()) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("does not exist on this table"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_a_later_generation_supersedes_the_one_recorded_for_that_shard() { + let manifest = backed_manifest(); + let (_, indices) = apply_with_indices( + &manifest, + vec![update(vec![(1, 3)])], + vec![empty_mem_wal_index()], + ) + .unwrap(); + let (_, indices) = + apply_with_indices(&manifest, vec![update(vec![(1, 9)])], indices).unwrap(); + + assert_eq!(progress(&indices), vec![(1, 9)]); + } + + #[test] + fn test_an_earlier_generation_is_rejected_rather_than_walking_a_shard_backwards() { + let manifest = backed_manifest(); + let (_, indices) = apply_with_indices( + &manifest, + vec![update(vec![(1, 9)])], + vec![empty_mem_wal_index()], + ) + .unwrap(); + let error = apply_with_indices(&manifest, vec![update(vec![(1, 3)])], indices).unwrap_err(); + + assert!( + error.to_string().contains("Stale SSTable compaction"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_shards_are_tracked_independently() { + let manifest = backed_manifest(); + let (_, indices) = apply_with_indices( + &manifest, + vec![update(vec![(1, 3)])], + vec![empty_mem_wal_index()], + ) + .unwrap(); + let (_, indices) = + apply_with_indices(&manifest, vec![update(vec![(2, 5)])], indices).unwrap(); + + assert_eq!(progress(&indices), vec![(1, 3), (2, 5)]); + } + + #[test] + fn test_recording_no_sstables_is_rejected() { + let error = apply_with_indices( + &backed_manifest(), + vec![update(vec![])], + vec![empty_mem_wal_index()], + ) + .unwrap_err(); + + assert!(matches!(error, Error::InvalidInput { .. }), "{error:?}"); + assert!( + error.to_string().contains("names no SSTable"), + "unexpected error: {error}" + ); + } + + #[test] + fn test_recording_progress_leaves_other_indices_alone() { + let kept = sample_index_metadata("by_a"); + let (_, indices) = apply_with_indices( + &backed_manifest(), + vec![update(vec![(1, 7)])], + vec![kept.clone(), empty_mem_wal_index()], + ) + .unwrap(); + + assert!(indices.iter().any(|index| index.uuid == kept.uuid)); + } + + #[test] + fn test_two_writers_recording_progress_conflict() { + let footprint = |actions| { + Footprint::from(&CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])) + }; + + let ours = footprint(vec![update(vec![(1, 3)])]); + let theirs = footprint(vec![update(vec![(2, 5)])]); + + assert!(ours.conflicts_with(&theirs)); + } +} diff --git a/rust/lance-table/src/transaction/conflicts.rs b/rust/lance-table/src/transaction/conflicts.rs index ad942d6c182..dc929bd9adf 100644 --- a/rust/lance-table/src/transaction/conflicts.rs +++ b/rust/lance-table/src/transaction/conflicts.rs @@ -863,6 +863,8 @@ impl PartialEq for Operation { std::mem::discriminant(self) == std::mem::discriminant(other) } (Self::DataOverlay { groups: a }, Self::DataOverlay { groups: b }) => compare_vec(a, b), + (Self::CompositeOperation(a), Self::CompositeOperation(b)) => a == b, + (Self::CompositeOperation(_), _) | (_, Self::CompositeOperation(_)) => false, (Self::DataOverlay { .. }, _) | (_, Self::DataOverlay { .. }) => false, } } diff --git a/rust/lance-table/src/transaction/manifest_build.rs b/rust/lance-table/src/transaction/manifest_build.rs index e47bc139e58..5390854faae 100644 --- a/rust/lance-table/src/transaction/manifest_build.rs +++ b/rust/lance-table/src/transaction/manifest_build.rs @@ -463,6 +463,16 @@ impl Transaction { config: &ManifestBuildConfig, read_version_state: Option>, ) -> Result<(Manifest, Vec)> { + if let Operation::CompositeOperation(composite_operation) = &self.operation { + return self.build_manifest_from_actions( + composite_operation, + current_manifest, + current_indices, + transaction_file_path, + config, + ); + } + if config.use_stable_row_ids && config.migration_next_row_id.is_none() && current_manifest @@ -1287,29 +1297,20 @@ impl Transaction { // Base paths are handled in the manifest creation section below final_fragments.extend(maybe_existing_fragments?.clone()); } + Operation::CompositeOperation(_) => { + // Handled by build_manifest_from_actions before this match. + return Err(Error::internal( + "an action-based operation reached the legacy manifest build".to_string(), + )); + } }; - // If a fragment was reserved then it may not belong at the end of the fragments list. - final_fragments.sort_by_key(|frag| frag.id); - - // Clean up data files that only contain tombstoned fields - Self::remove_tombstoned_data_files(&mut final_fragments); + Self::normalize_fragments(&mut final_fragments)?; - // Enforce the newest-last overlay ordering invariant at the write - // boundary. Load normalizes with a sort; this rejects any commit path - // that assembled a fragment's overlays out of order. - for fragment in &final_fragments { - if !fragment.overlays.is_empty() { - crate::format::overlay::verify_overlays_newest_last(&fragment.overlays)?; - } - } - - let user_requested_version = match (&config.storage_format, config.use_legacy_format) { - (Some(storage_format), _) => Some(storage_format.lance_file_format()), - (None, Some(true)) => Some(ConcreteFileVersion::V1), - (None, Some(false)) => Some(ConcreteFileVersion::V2_0), - (None, None) => None, - }; + // If this is an overwrite operation and the user has requested a specific + // version then overwrite with that version. Otherwise, if the user didn't + // request a specific version, then keep whatever version we had before. + let overwrite_storage_format = matches!(self.operation, Operation::Overwrite { .. }); // Applied once the final index list is known, so it sees exactly the // indices this commit publishes rather than what any one operation arm @@ -1325,55 +1326,14 @@ impl Transaction { )?; } - let mut manifest = if let Some(current_manifest) = current_manifest { - // OVERWRITE with initial_bases on existing dataset is not allowed (caught by validation) - // So we always use new_from_previous which preserves base_paths - let mut prev_manifest = - Manifest::new_from_previous(current_manifest, schema, Arc::new(final_fragments)); - - if let (Some(user_requested_version), Operation::Overwrite { .. }) = - (user_requested_version, &self.operation) - { - // If this is an overwrite operation and the user has requested a specific version - // then overwrite with that version. Otherwise, if the user didn't request a specific - // version, then overwrite with whatever version we had before. - prev_manifest.data_storage_format = DataStorageFormat::new(user_requested_version); - } - - prev_manifest - } else { - let data_storage_format = - Self::data_storage_format_from_files(&final_fragments, user_requested_version)?; - Manifest::new( - schema, - Arc::new(final_fragments), - data_storage_format, - reference_paths, - ) - }; - - manifest.tag.clone_from(&self.tag); - - if config.auto_set_feature_flags { - // Internal operations (e.g. CreateIndex) build with the default config, - // which has use_stable_row_ids = false. Without inheriting from the previous - // manifest, apply_feature_flags would clear FLAG_STABLE_ROW_IDS. - let inherited = current_manifest - .map(|m| m.uses_stable_row_ids()) - .unwrap_or(false); - let use_stable_row_ids = config.use_stable_row_ids || inherited; - apply_feature_flags( - &mut manifest, - use_stable_row_ids, - config.disable_transaction_file, - )?; - } - // Carried from the manifest this one is derived from. `new_from_previous` - // zeroes both feature words, so `apply_feature_flags` cannot see the - // previous state and every ordinary commit would otherwise drop the bit. - if let Some(current_manifest) = current_manifest { - inherit_mem_wal_index_catchup(&mut manifest, current_manifest)?; - } + let mut manifest = self.assemble_manifest( + current_manifest, + schema, + final_fragments, + reference_paths, + overwrite_storage_format, + config, + )?; // Set after apply_feature_flags, which resets both flag words: activation // is the one place the bit is turned on, and it must survive that reset. @@ -1413,10 +1373,6 @@ impl Transaction { manifest.writer_feature_flags |= FLAG_MEM_WAL_INDEX_CATCHUP; } - manifest.set_timestamp(config.timestamp_nanos); - - manifest.update_max_fragment_id(); - match &self.operation { Operation::Overwrite { config_upsert_values: Some(tm), @@ -1599,6 +1555,106 @@ impl Transaction { Ok((manifest, final_indices)) } + /// Put an assembled fragment list into the shape the manifest requires: + /// ordered by id, with no data file left holding only tombstoned fields, and + /// with each fragment's overlays in oldest-to-newest order. + pub(super) fn normalize_fragments(fragments: &mut [Fragment]) -> Result<()> { + // If a fragment was reserved then it may not belong at the end of the list. + fragments.sort_by_key(|frag| frag.id); + + Self::remove_tombstoned_data_files(fragments); + + // Enforce the newest-last overlay ordering invariant at the write + // boundary. Load normalizes with a sort; this rejects any commit path + // that assembled a fragment's overlays out of order. + for fragment in fragments.iter() { + if !fragment.overlays.is_empty() { + crate::format::overlay::verify_overlays_newest_last(&fragment.overlays)?; + } + } + Ok(()) + } + + fn user_requested_version(config: &ManifestBuildConfig) -> Option { + match (&config.storage_format, config.use_legacy_format) { + (Some(storage_format), _) => Some(storage_format.lance_file_format()), + (None, Some(true)) => Some(ConcreteFileVersion::V1), + (None, Some(false)) => Some(ConcreteFileVersion::V2_0), + (None, None) => None, + } + } + + /// Build the manifest itself from an already-decided schema and fragment + /// list, and apply the settings that every operation shares: the tag, + /// feature flags, timestamp, and fragment id watermark. + /// + /// `overwrite_storage_format` replaces the inherited storage format with the + /// user-requested one, for operations that rewrite the whole dataset. + pub(super) fn assemble_manifest( + &self, + current_manifest: Option<&Manifest>, + schema: lance_core::datatypes::Schema, + fragments: Vec, + reference_paths: HashMap, + overwrite_storage_format: bool, + config: &ManifestBuildConfig, + ) -> Result { + let user_requested_version = Self::user_requested_version(config); + + let mut manifest = if let Some(current_manifest) = current_manifest { + // OVERWRITE with initial_bases on existing dataset is not allowed (caught by validation) + // So we always use new_from_previous which preserves base_paths + let mut prev_manifest = + Manifest::new_from_previous(current_manifest, schema, Arc::new(fragments)); + + if let (true, Some(user_requested_version)) = + (overwrite_storage_format, user_requested_version) + { + prev_manifest.data_storage_format = DataStorageFormat::new(user_requested_version); + } + + prev_manifest + } else { + let data_storage_format = + Self::data_storage_format_from_files(&fragments, user_requested_version)?; + Manifest::new( + schema, + Arc::new(fragments), + data_storage_format, + reference_paths, + ) + }; + + manifest.tag.clone_from(&self.tag); + + if config.auto_set_feature_flags { + // Internal operations (e.g. CreateIndex) build with the default config, + // which has use_stable_row_ids = false. Without inheriting from the previous + // manifest, apply_feature_flags would clear FLAG_STABLE_ROW_IDS. + let inherited = current_manifest + .map(|m| m.uses_stable_row_ids()) + .unwrap_or(false); + let use_stable_row_ids = config.use_stable_row_ids || inherited; + apply_feature_flags( + &mut manifest, + use_stable_row_ids, + config.disable_transaction_file, + )?; + } + // Carried from the manifest this one is derived from. `new_from_previous` + // zeroes both feature words, so `apply_feature_flags` cannot see the + // previous state and every ordinary commit would otherwise drop the bit. + if let Some(current_manifest) = current_manifest { + inherit_mem_wal_index_catchup(&mut manifest, current_manifest)?; + } + + manifest.set_timestamp(config.timestamp_nanos); + + manifest.update_max_fragment_id(); + + Ok(manifest) + } + /// Remove data files that only contain tombstoned fields (-2) /// These files no longer contain any live data and can be safely dropped fn remove_tombstoned_data_files(fragments: &mut [Fragment]) { diff --git a/rust/lance-table/src/transaction/operation.rs b/rust/lance-table/src/transaction/operation.rs index 1874984864b..df6356630a2 100644 --- a/rust/lance-table/src/transaction/operation.rs +++ b/rust/lance-table/src/transaction/operation.rs @@ -14,6 +14,7 @@ use crate::format::overlay::DataOverlayFile; use crate::format::{BasePath, DataFile, Fragment, IndexFile, IndexMetadata}; use crate::system_index::mem_wal::CompactedSsTable; use crate::transaction::UpdateMap; +use crate::transaction::action::CompositeOperation; use lance_core::datatypes::Schema; use lance_core::deepsize::DeepSizeOf; use roaring::RoaringBitmap; @@ -220,6 +221,14 @@ pub enum Operation { /// The new base paths to add to the manifest. new_bases: Vec, }, + + /// A Transaction V2 operation: an ordered list of granular actions that + /// commit atomically as one manifest change. + /// + /// Unlike the variants above, this one is not a single named change -- it is + /// the composable form the others decompose into. See + /// [`super::action`] for the vocabulary and its stability caveats. + CompositeOperation(CompositeOperation), } #[derive(Debug, Clone, PartialEq, DeepSizeOf)] @@ -270,6 +279,7 @@ impl std::fmt::Display for Operation { Self::Clone { .. } => write!(f, "Clone"), Self::UpdateMemWalState { .. } => write!(f, "UpdateMemWalState"), Self::UpdateBases { .. } => write!(f, "UpdateBases"), + Self::CompositeOperation(_) => write!(f, "CompositeOperation"), } } } @@ -329,6 +339,7 @@ impl Operation { Self::UpdateMemWalState { .. } => "UpdateMemWalState", Self::Clone { .. } => "Clone", Self::UpdateBases { .. } => "UpdateBases", + Self::CompositeOperation(_) => "CompositeOperation", } } } diff --git a/rust/lance-table/src/transaction/proto.rs b/rust/lance-table/src/transaction/proto.rs index 5082ec8b57e..1ef126932d8 100644 --- a/rust/lance-table/src/transaction/proto.rs +++ b/rust/lance-table/src/transaction/proto.rs @@ -12,6 +12,7 @@ use crate::format::key_existence::KeyExistenceFilter; use crate::format::pb; use crate::format::{BasePath, Fragment, IndexFile, IndexMetadata, overlay::DataOverlayFile}; use crate::system_index::mem_wal::CompactedSsTable; +use crate::transaction::action::CompositeOperation; use crate::transaction::{ DataOverlayGroup, DataReplacementGroup, Operation, RewriteGroup, RewrittenIndex, Transaction, UpdateMap, UpdateMapEntry, UpdateMode, UpdatedFragmentOffsets, translate_config_updates, @@ -416,18 +417,14 @@ impl TryFrom for Transaction { .map(DataOverlayGroup::try_from) .collect::>>()?, }, - Some(pb::transaction::Operation::UserOperation(_)) => { + Some(pb::transaction::Operation::CompositeOperation(composite_operation)) => { // Action-based transactions (Transaction V2) are a draft wire - // format (OSS-1530). This version of Lance recognizes the message - // but has no support for it: reject on load, fail-closed. Because - // load_and_sort_new_transactions collects transactions with - // try_collect, a concurrent V2 commit in the conflict window - // aborts the whole commit rather than being silently skipped. - // Do NOT make this parsing lenient. - return Err(Error::not_supported( - "action-based transactions (Transaction V2) are not supported \ - by this version of Lance; please upgrade", - )); + // format (OSS-1530). Parsing is fail-closed: an action this build + // does not implement is an error, never a skipped element. + // load_and_sort_new_transactions collects concurrent transactions + // with try_collect, so such a transaction aborts the commit rather + // than being silently treated as a no-op. Do NOT make this lenient. + Operation::CompositeOperation(CompositeOperation::try_from(composite_operation)?) } None => { return Err(Error::internal( @@ -732,6 +729,15 @@ impl From<&Transaction> for pb::Transaction { .collect::>(), }) } + Operation::CompositeOperation(composite_operation) => { + let mut message = pb::CompositeOperation::from(composite_operation); + // The operation's identity and read version are the enclosing + // transaction's; the wire carries them in both places so a + // squashed operation keeps its own provenance. + message.uuid = value.uuid.clone(); + message.read_version = value.read_version; + pb::transaction::Operation::CompositeOperation(message) + } }; let transaction_properties = value @@ -836,6 +842,7 @@ mod tests { use super::*; use crate::format::DataFile; use crate::format::overlay::OverlayCoverage; + use crate::transaction::action::{Action, AddFragment, UserAction}; #[test] fn test_data_overlay_operation_roundtrips() { @@ -882,29 +889,60 @@ mod tests { } #[test] - fn test_user_operation_rejected_on_load() { - // Action-based transactions (Transaction V2) are a draft wire format that - // this version of Lance does not support. Loading one must fail closed - // (never be silently skipped or leniently parsed), so that a concurrent - // V2 commit in the conflict window aborts an in-flight commit. + fn test_composite_operation_round_trips_through_transaction() { + let uuid = Uuid::new_v4().to_string(); + let transaction = Transaction { + read_version: 4, + uuid: uuid.clone(), + operation: Operation::CompositeOperation(CompositeOperation::new(vec![ + UserAction::new( + "append batch", + vec![Action::AddFragment(AddFragment { + local: 0, + physical_rows: 1, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + })], + ), + ])), + tag: None, + transaction_properties: None, + }; + + let message = pb::Transaction::from(&transaction); + // The operation repeats the envelope's identity so a squashed operation + // keeps the provenance of the commit it came from. + match &message.operation { + Some(pb::transaction::Operation::CompositeOperation(composite_operation)) => { + assert_eq!(composite_operation.uuid, uuid); + assert_eq!(composite_operation.read_version, 4); + } + other => panic!("expected CompositeOperation, got {other:?}"), + } + + assert_eq!(Transaction::try_from(message).unwrap(), transaction); + } + + #[test] + fn test_unrecognized_action_fails_closed_on_load() { + // An action a newer Lance wrote decodes to no known variant, because + // protobuf drops the field this build does not know. Loading it must + // fail rather than parse leniently: load_and_sort_new_transactions + // collects concurrent transactions with try_collect, so this aborts an + // in-flight commit instead of letting it proceed against a change it + // cannot see. let message = pb::Transaction { read_version: 1, uuid: Uuid::new_v4().to_string(), - operation: Some(pb::transaction::Operation::UserOperation( - pb::UserOperation { - description: "INSERT INTO t VALUES (1)".to_string(), + operation: Some(pb::transaction::Operation::CompositeOperation( + pb::CompositeOperation { uuid: Uuid::new_v4().to_string(), read_version: 1, actions: vec![pb::UserAction { - description: "append batch".to_string(), - actions: vec![pb::Action { - action: Some(pb::action::Action::AddFragment(pb::AddFragment { - local: 0, - physical_rows: 1, - data_change: Some(true), - ..Default::default() - })), - }], + description: "something from the future".to_string(), + actions: vec![pb::Action { action: None }], }], }, )), diff --git a/rust/lance/src/dataset/tests/dataset_transactions.rs b/rust/lance/src/dataset/tests/dataset_transactions.rs index 5b9c76bc90c..fcb89459b83 100644 --- a/rust/lance/src/dataset/tests/dataset_transactions.rs +++ b/rust/lance/src/dataset/tests/dataset_transactions.rs @@ -1395,3 +1395,638 @@ async fn test_alter_columns_materializes_fresh_field_id_in_every_fragment() { let batch = dataset.scan().try_into_batch().await.unwrap(); assert_eq!(batch["a"].as_primitive::().values(), &[1, 2]); } + +mod composite { + //! End-to-end coverage for committing an action set against a real dataset. + //! + //! Each of these is a single commit that does work a named operation would + //! have needed several commits for: a fragment is added and then modified, a + //! field is added and then filled, all inside one version. That is what the + //! action vocabulary buys -- steps that reference each other's minted ids can + //! be squashed into one atomic manifest change. + //! + //! The commit path checks that a referenced data file exists, so these tests + //! point their actions at files the fixture dataset already wrote. The + //! resulting datasets are inspected through their manifests rather than read -- + //! the files hold the wrong columns for where they end up attached. + + use std::sync::Arc; + + use crate::Dataset; + use crate::dataset::{CommitBuilder, InsertBuilder, WriteParams}; + use crate::index::DatasetIndexExt; + use arrow_array::{Int32Array, RecordBatch}; + use arrow_schema::{DataType, Field, Schema}; + use lance_table::format::DataFile; + use lance_table::format::key_existence::{FilterType, KeyExistenceFilter}; + use lance_table::format::overlay::{DataOverlayFile, OverlayCoverage}; + use lance_table::system_index::mem_wal::{ + CompactedSsTable, MEM_WAL_INDEX_NAME, MemWalIndexDetails, load_mem_wal_index_details, + new_mem_wal_index_meta, + }; + use lance_table::transaction::action::{ + Action, AddDataFile, AddField, AddFragment, AddIndexSegment, AddOverlays, + AdjustIndexCoverage, AssertUniqueKeys, CompositeOperation, DropField, Ref, + RefreshRowVersionMetadata, RemoveIndexSegment, TombstoneFieldData, UpdateCompactedSsTables, + UserAction, + }; + use lance_table::transaction::{Operation, Transaction}; + use uuid::Uuid; + + /// A two-fragment dataset, so its two data files can stand in for the files an + /// action set would otherwise have had to write. + async fn test_dataset(enable_stable_row_ids: bool) -> Dataset { + let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int32, false)])); + let data = + RecordBatch::try_new(schema, vec![Arc::new(Int32Array::from_iter_values(0..10))]) + .unwrap(); + + InsertBuilder::new("memory://") + .with_params(&WriteParams { + enable_stable_row_ids, + max_rows_per_file: 5, + ..Default::default() + }) + .execute(vec![data]) + .await + .unwrap() + } + + fn existing_data_file(dataset: &Dataset, fragment: usize) -> DataFile { + dataset.fragments()[fragment].files[0].clone() + } + + async fn commit(dataset: Dataset, actions: Vec) -> Dataset { + let read_version = dataset.version().version; + CommitBuilder::new(Arc::new(dataset)) + .execute(Transaction::new( + read_version, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])), + None, + )) + .await + .unwrap() + } + + #[tokio::test] + async fn test_one_commit_adds_a_fragment_and_then_modifies_it() { + let dataset = test_dataset(false).await; + let before = dataset.version().version; + let first_file = existing_data_file(&dataset, 0); + let second_file = existing_data_file(&dataset, 1); + let second_path = second_file.path.clone(); + + let dataset = commit( + dataset, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: first_file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + // The same commit then replaces the data it just added, naming the + // fragment by the token it was minted under. + Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Local(0), + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file: second_file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + ], + ) + .await; + + assert_eq!(dataset.version().version, before + 1); + let fragments = dataset.fragments(); + assert_eq!(fragments.len(), 3); + let added = fragments.last().unwrap(); + assert_eq!(added.physical_rows, Some(4)); + // The tombstoned file is gone; only the replacement survives the commit. + let paths = added + .files + .iter() + .map(|file| file.path.clone()) + .collect::>(); + assert_eq!(paths, vec![second_path]); + } + + #[tokio::test] + async fn test_one_commit_adds_a_field_and_then_fills_it() { + let dataset = test_dataset(false).await; + let fragment_id = dataset.fragments()[0].id; + let file = existing_data_file(&dataset, 1); + let path = file.path.clone(); + + let dataset = commit( + dataset, + vec![ + Action::AddField(AddField { + local: 0, + parent: None, + def: lance_core::datatypes::Field::try_from(Field::new( + "b", + DataType::Int32, + true, + )) + .unwrap(), + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(fragment_id), + file, + field_ids: vec![Ref::Local(0)], + data_change: true, + }), + ], + ) + .await; + + let field = dataset.schema().field("b").expect("field b was added"); + let fragment = &dataset.fragments()[0]; + let added = fragment + .files + .iter() + .find(|file| file.path == path) + .expect("the new field's data file was attached"); + // The file points at the id the commit minted, which the caller never knew. + assert_eq!(added.fields.as_ref(), &[field.id]); + } + + #[tokio::test] + async fn test_one_commit_swaps_a_field_for_a_new_one() { + let dataset = test_dataset(false).await; + let fragment_id = dataset.fragments()[0].id; + let file = existing_data_file(&dataset, 1); + + let dataset = commit( + dataset, + vec![ + Action::DropField(DropField { + field: Ref::Committed(0), + }), + Action::AddField(AddField { + local: 0, + parent: None, + def: lance_core::datatypes::Field::try_from(Field::new( + "a", + DataType::Int64, + true, + )) + .unwrap(), + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(fragment_id), + file, + field_ids: vec![Ref::Local(0)], + data_change: true, + }), + ], + ) + .await; + + // Dropping "a" and adding a new "a" of a different type is one version, and + // the new field gets a fresh id rather than inheriting the dropped one. + let schema = dataset.schema(); + assert_eq!(schema.fields.len(), 1); + let field = schema.field("a").unwrap(); + assert_ne!(field.id, 0); + assert_eq!(field.logical_type.to_string(), "int64"); + } + + #[tokio::test] + async fn test_one_commit_assigns_row_ids_to_the_fragments_it_mints() { + let dataset = test_dataset(true).await; + let next_row_id = dataset.manifest().next_row_id; + assert_eq!(next_row_id, 10); + + let dataset = commit( + dataset, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddFragment(AddFragment { + local: 1, + physical_rows: 6, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + ], + ) + .await; + + assert_eq!(dataset.manifest().next_row_id, 20); + for fragment in dataset.fragments().iter().skip(2) { + assert!( + fragment.row_id_meta.is_some(), + "fragment {} was minted without row ids", + fragment.id + ); + assert!(fragment.created_at_version_meta.is_some()); + } + } + + #[tokio::test] + async fn test_two_action_sets_on_disjoint_coordinates_both_commit() { + let dataset = Arc::new(test_dataset(false).await); + let read_version = dataset.version().version; + + let append = |local| { + vec![Action::AddFragment(AddFragment { + local, + physical_rows: 4, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + })] + }; + + let first = CommitBuilder::new(dataset.clone()) + .execute(Transaction::new( + read_version, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", + append(0), + )])), + None, + )) + .await + .unwrap(); + + // The second commit still reads the original version, so it has to be + // checked against the first. Both only mint, so neither writes anything + // the other does. + let second = CommitBuilder::new(dataset) + .execute(Transaction::new( + read_version, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", + append(0), + )])), + None, + )) + .await + .unwrap(); + + assert_eq!(second.version().version, first.version().version + 1); + assert_eq!(second.fragments().len(), 4); + } + + #[tokio::test] + async fn test_two_action_sets_writing_the_same_field_data_conflict() { + let dataset = Arc::new(test_dataset(false).await); + let read_version = dataset.version().version; + let fragment_id = dataset.fragments()[0].id; + + let tombstone = || { + Transaction::new( + read_version, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", + vec![Action::TombstoneFieldData(TombstoneFieldData { + fragment: Ref::Committed(fragment_id), + field_ids: vec![Ref::Committed(0)], + data_change: true, + })], + )])), + None, + ) + }; + + CommitBuilder::new(dataset.clone()) + .execute(tombstone()) + .await + .unwrap(); + + let error = CommitBuilder::new(dataset) + .with_max_retries(0) + .execute(tombstone()) + .await + .unwrap_err(); + assert!( + error.to_string().contains("preempted"), + "unexpected error: {error}" + ); + } + + /// An index segment naming `covered`, with no files of its own -- these + /// tests inspect the manifest's index section rather than opening the index. + fn index_segment(name: &str, covered: Vec) -> AddIndexSegment { + AddIndexSegment { + uuid: Uuid::new_v4(), + name: name.into(), + fields: vec![Ref::Committed(0)], + index_details: None, + index_version: 1, + covered_fragments: Some(covered), + files: Vec::new(), + base: None, + created_at: None, + dataset_version: None, + data_change: false, + } + } + + async fn index_coverage(dataset: &Dataset, name: &str) -> Vec { + let indices = dataset.load_indices().await.unwrap(); + let segment = indices + .iter() + .find(|index| index.name == name) + .expect("index segment should be committed"); + segment + .fragment_bitmap + .as_ref() + .expect("coverage should be recorded") + .iter() + .collect() + } + + #[tokio::test] + async fn test_one_commit_appends_and_indexes_what_it_appended() { + let dataset = test_dataset(false).await; + let existing = dataset.fragments()[0].id; + let file = existing_data_file(&dataset, 0); + + let dataset = commit( + dataset, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 5, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + // The index covers the fragment this same commit minted, which + // has no id until the commit lands. + Action::AddIndexSegment(index_segment( + "by_a", + vec![Ref::Committed(existing), Ref::Local(0)], + )), + ], + ) + .await; + + let appended = dataset.fragments().last().unwrap().id; + assert_eq!( + index_coverage(&dataset, "by_a").await, + vec![existing as u32, appended as u32] + ); + } + + #[tokio::test] + async fn test_one_commit_replaces_an_index_segment() { + let dataset = test_dataset(false).await; + let old = index_segment("by_a", vec![Ref::Committed(0)]); + let old_uuid = old.uuid; + let dataset = commit(dataset, vec![Action::AddIndexSegment(old)]).await; + + let new = index_segment("by_a", vec![Ref::Committed(0), Ref::Committed(1)]); + let new_uuid = new.uuid; + let dataset = commit( + dataset, + vec![ + Action::RemoveIndexSegment(RemoveIndexSegment { + uuid: old_uuid, + data_change: false, + }), + Action::AddIndexSegment(new), + ], + ) + .await; + + let uuids = dataset + .load_indices() + .await + .unwrap() + .iter() + .map(|index| index.uuid) + .collect::>(); + assert_eq!(uuids, vec![new_uuid]); + } + + #[tokio::test] + async fn test_a_commit_extends_an_index_segments_coverage() { + let dataset = test_dataset(false).await; + let segment = index_segment("by_a", vec![Ref::Committed(0)]); + let uuid = segment.uuid; + let dataset = commit(dataset, vec![Action::AddIndexSegment(segment)]).await; + assert_eq!(index_coverage(&dataset, "by_a").await, vec![0]); + + let dataset = commit( + dataset, + vec![Action::AdjustIndexCoverage(AdjustIndexCoverage { + uuid, + add_fragments: vec![Ref::Committed(1)], + remove_fragments: vec![0], + })], + ) + .await; + + assert_eq!(index_coverage(&dataset, "by_a").await, vec![1]); + } + + #[tokio::test] + async fn test_one_commit_appends_and_overlays_what_was_already_there() { + let dataset = test_dataset(false).await; + let overlaid = dataset.fragments()[0].id; + let file = existing_data_file(&dataset, 0); + let overlay_file = existing_data_file(&dataset, 1); + let expected_version = dataset.version().version + 1; + + let dataset = commit( + dataset, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 5, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + Action::AddOverlays(AddOverlays { + fragment: Ref::Committed(overlaid), + overlays: vec![DataOverlayFile { + data_file: overlay_file, + coverage: OverlayCoverage::dense([0u32, 2].into_iter().collect()), + // Left for the commit to stamp. + committed_version: 0, + }], + data_change: true, + }), + ], + ) + .await; + + assert_eq!(dataset.fragments().len(), 3); + let fragment = dataset + .fragments() + .iter() + .find(|fragment| fragment.id == overlaid) + .unwrap(); + assert_eq!(fragment.overlays.len(), 1); + assert_eq!(fragment.overlays[0].committed_version, expected_version); + } + + #[tokio::test] + async fn test_a_commit_restamps_row_versions_for_a_fragment_it_rewrote() { + let dataset = test_dataset(true).await; + let rewritten = dataset.fragments()[0].id; + let file = existing_data_file(&dataset, 1); + let expected_version = dataset.version().version + 1; + + let dataset = commit( + dataset, + vec![ + // Rewriting a column in place leaves the rows where they are, so + // nothing else in the commit says when they last changed. + Action::AddDataFile(AddDataFile { + fragment: Ref::Committed(rewritten), + file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + Action::RefreshRowVersionMetadata(RefreshRowVersionMetadata { + fragment_ids: vec![rewritten], + }), + ], + ) + .await; + + let fragment = dataset + .fragments() + .iter() + .find(|fragment| fragment.id == rewritten) + .unwrap(); + let sequence = fragment + .last_updated_at_version_meta + .as_ref() + .expect("the rewritten fragment should carry a last-updated sequence") + .load_sequence() + .unwrap(); + let versions = (0..fragment.physical_rows.unwrap()) + .map(|offset| sequence.version_at(offset).unwrap()) + .collect::>(); + assert_eq!(versions, vec![expected_version; versions.len()]); + } + + #[tokio::test] + async fn test_a_commit_records_mem_wal_compaction_progress() { + let dataset = test_dataset(false).await; + let shard = Uuid::new_v4(); + + // Progress is only recordable against a table that already carries the + // index, so put one there the way the MemWAL writer would. + let read_version = dataset.version().version; + let dataset = CommitBuilder::new(Arc::new(dataset)) + .execute(Transaction::new( + read_version, + Operation::CreateIndex { + new_indices: vec![ + new_mem_wal_index_meta(read_version, MemWalIndexDetails::default()) + .unwrap(), + ], + removed_indices: Vec::new(), + }, + None, + )) + .await + .unwrap(); + + let dataset = commit( + dataset, + vec![Action::UpdateCompactedSsTables(UpdateCompactedSsTables { + compacted_sstables: vec![CompactedSsTable::new(shard, 3)], + })], + ) + .await; + + let indices = dataset.load_indices().await.unwrap(); + let mem_wal = indices + .iter() + .find(|index| index.name == MEM_WAL_INDEX_NAME) + .expect("the MemWAL index should still be there"); + let details = load_mem_wal_index_details((*mem_wal).clone()).unwrap(); + assert_eq!(details.compacted_sstables.len(), 1); + assert_eq!(details.compacted_sstables[0].shard_id, shard); + assert_eq!(details.compacted_sstables[0].generation, 3); + + // The data is untouched: recording where rows live is not a change to + // them. + assert_eq!(dataset.fragments().len(), 2); + } + + #[tokio::test] + async fn test_a_commit_carrying_a_key_assertion_changes_nothing_itself() { + let dataset = test_dataset(false).await; + let file = existing_data_file(&dataset, 0); + let before = dataset.fragments().len(); + + let dataset = commit( + dataset, + vec![ + Action::AddFragment(AddFragment { + local: 0, + physical_rows: 5, + row_id_meta: None, + last_updated_at_version_meta: None, + created_at_version_meta: None, + data_change: true, + }), + Action::AddDataFile(AddDataFile { + fragment: Ref::Local(0), + file, + field_ids: vec![Ref::Committed(0)], + data_change: true, + }), + Action::AssertUniqueKeys(AssertUniqueKeys { + key_fields: vec![Ref::Committed(0)], + filter: KeyExistenceFilter { + field_ids: Vec::new(), + filter: FilterType::ExactSet([11u64, 12].into_iter().collect()), + }, + }), + ], + ) + .await; + + assert_eq!(dataset.fragments().len(), before + 1); + } +} diff --git a/rust/lance/src/io/commit/conflict_resolver.rs b/rust/lance/src/io/commit/conflict_resolver.rs index 99b5188f91b..de6f7ee29e2 100644 --- a/rust/lance/src/io/commit/conflict_resolver.rs +++ b/rust/lance/src/io/commit/conflict_resolver.rs @@ -16,6 +16,7 @@ use lance_index::mem_wal::{CompactedSsTable, MEM_WAL_INDEX_NAME}; use lance_select::{RowAddrTreeMap, RowSetOps}; use lance_table::format::IndexMetadata; use lance_table::format::overlay::OverlayCoverage; +use lance_table::transaction::action::{CompositeOperation, Footprint, UserAction}; use lance_table::{format::Fragment, io::deletion::write_deletion_file}; use roaring::RoaringBitmap; use std::{ @@ -89,7 +90,11 @@ impl<'a> TransactionRebase<'a> { | Operation::UpdateMemWalState { .. } | Operation::Clone { .. } | Operation::Restore { .. } - | Operation::UpdateBases { .. } => Ok(Self { + | Operation::UpdateBases { .. } + // An action set can modify fragments, but conflicts against it are + // settled by comparing footprints, which are derived from the + // actions rather than from collected rebase state. + | Operation::CompositeOperation(_) => Ok(Self { transaction, affected_rows, initial_fragments: HashMap::new(), @@ -314,7 +319,34 @@ impl<'a> TransactionRebase<'a> { Operation::UpdateBases { .. } => { self.check_add_bases_txn(other_transaction, other_version) } + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } + } + } + + /// Whether an action-based transaction conflicts with `other_transaction`. + /// + /// Reached from both directions: when the transaction being committed is + /// action-based, and when a concurrent one is. + fn check_action_txn( + &mut self, + other_transaction: &Transaction, + other_version: u64, + ) -> Result<()> { + let (Some(ours), Some(theirs)) = ( + footprint_of(&self.transaction.operation), + footprint_of(&other_transaction.operation), + ) else { + // One side does not decompose into actions yet, so there is nothing + // to compare and the conservative answer stands. + return Err(self.retryable_conflict_err(other_transaction, other_version)); + }; + + if ours.conflicts_with(&theirs) { + return Err(self.retryable_conflict_err(other_transaction, other_version)); } + Ok(()) } fn check_delete_txn( @@ -324,6 +356,11 @@ impl<'a> TransactionRebase<'a> { ) -> Result<()> { if let Operation::Delete { .. } = &self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::CreateIndex { .. } | Operation::ReserveFragments { .. } | Operation::Clone { .. } @@ -478,6 +515,11 @@ impl<'a> TransactionRebase<'a> { } match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::CreateIndex { .. } | Operation::ReserveFragments { .. } | Operation::Project { .. } @@ -641,6 +683,11 @@ impl<'a> TransactionRebase<'a> { } = &mut self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Append { .. } | Operation::Clone { .. } // An overlay committed after this index's version is newer than @@ -840,6 +887,11 @@ impl<'a> TransactionRebase<'a> { } = &self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } // Rewrite is only compatible with operations that don't touch // existing fragments or update fragments we don't touch. Operation::Append { .. } @@ -1030,6 +1082,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Overwrite { .. } => { if self .transaction @@ -1079,6 +1136,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } // Append is not compatible with any operation that completely // overwrites the schema. Operation::Overwrite { .. } @@ -1109,6 +1171,11 @@ impl<'a> TransactionRebase<'a> { ) -> Result<()> { if let Operation::DataReplacement { replacements } = &self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Append { .. } | Operation::Clone { .. } | Operation::UpdateConfig { .. } @@ -1287,6 +1354,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Append { .. } | Operation::CreateIndex { .. } | Operation::ReserveFragments { .. } @@ -1385,6 +1457,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } // See the MemWAL exception in check_create_index_txn. Operation::CreateIndex { new_indices, .. } => { if new_indices.iter().any(|idx| idx.name == MEM_WAL_INDEX_NAME) { @@ -1422,6 +1499,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Append { .. } | Operation::Delete { .. } | Operation::Overwrite { .. } @@ -1449,6 +1531,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Overwrite { .. } | Operation::Restore { .. } => { Err(self.incompatible_conflict_err(other_transaction, other_version)) } @@ -1475,6 +1562,11 @@ impl<'a> TransactionRebase<'a> { other_version: u64, ) -> Result<()> { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } // Project is compatible with anything that doesn't change the schema Operation::Append { .. } | Operation::Update { .. } @@ -1511,6 +1603,11 @@ impl<'a> TransactionRebase<'a> { } = &self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::Overwrite { .. } => { // Updates to schema metadata or field metadata conflict with any kind // of overwrite. @@ -1574,6 +1671,11 @@ impl<'a> TransactionRebase<'a> { } = &self.transaction.operation { match &other_transaction.operation { + // A concurrent action-based transaction is compared by action + // footprint rather than by operation pair. + Operation::CompositeOperation(_) => { + self.check_action_txn(other_transaction, other_version) + } Operation::UpdateMemWalState { compacted_sstables: other_compacted_sstables, .. @@ -1727,7 +1829,12 @@ impl<'a> TransactionRebase<'a> { | Operation::Clone { .. } | Operation::UpdateConfig { .. } | Operation::UpdateMemWalState { .. } - | Operation::UpdateBases { .. } => Ok(self.transaction), + | Operation::UpdateBases { .. } + // An action set needs no rewriting to move to a newer version: its + // minted ids are allocated when the actions are applied, against + // whichever manifest they land on, and its committed references + // name coordinates that do not move. + | Operation::CompositeOperation(_) => Ok(self.transaction), } } @@ -2238,6 +2345,23 @@ fn overlay_group_coverage(group: &DataOverlayGroup) -> RoaringBitmap { union } +/// The set of coordinates an operation writes, or `None` when it does not +/// decompose into actions yet. +/// +/// A legacy operation gets a footprint through its action translation, so an +/// action set can be compared against a concurrent named operation without +/// either side needing an entry in the operation-pair matrix. +fn footprint_of(operation: &Operation) -> Option { + match operation { + Operation::CompositeOperation(composite_operation) => { + Some(Footprint::from(composite_operation)) + } + other => Vec::::try_from(other) + .ok() + .map(|actions| Footprint::from(&CompositeOperation::new(actions))), + } +} + fn wrong_operation_err(op: &Operation) -> Error { Error::internal(format!("function called against a wrong operation: {}", op)) } @@ -2256,6 +2380,9 @@ mod tests { use lance_table::format::IndexMetadata; use lance_table::io::deletion::{deletion_file_path, read_deletion_file}; + use lance_table::transaction::action::{ + Action as TxnAction, Ref as ActionRef, RemoveFragment, TombstoneFieldData, + }; use super::*; use crate::dataset::transaction::{DataReplacementGroup, RewriteGroup}; @@ -4158,6 +4285,181 @@ mod tests { assert!(rebase.check_txn(&txn2, 2).is_ok()); } + fn action_txn(actions: Vec) -> Transaction { + Transaction::new_from_version( + 1, + Operation::CompositeOperation(CompositeOperation::new(vec![UserAction::new( + "step", actions, + )])), + ) + } + + fn tombstone_txn(fragment: u64, field: u64) -> Transaction { + action_txn(vec![TxnAction::TombstoneFieldData(TombstoneFieldData { + fragment: ActionRef::Committed(fragment), + field_ids: vec![ActionRef::Committed(field)], + data_change: true, + })]) + } + + /// Assert the verdict holds whichever transaction is the one rebasing. + async fn assert_conflict(dataset: &Dataset, a: Transaction, b: Transaction, expected: bool) { + for (ours, theirs) in [(a.clone(), b.clone()), (b, a)] { + let mut rebase = TransactionRebase::try_new(dataset, ours, None) + .await + .unwrap(); + assert_eq!(rebase.check_txn(&theirs, 2).is_err(), expected); + } + } + + #[tokio::test] + async fn test_action_txns_on_disjoint_coordinates_do_not_conflict() { + let dataset = test_dataset(10, 2).await; + assert_conflict(&dataset, tombstone_txn(0, 0), tombstone_txn(1, 0), false).await; + assert_conflict(&dataset, tombstone_txn(0, 0), tombstone_txn(0, 1), false).await; + } + + #[tokio::test] + async fn test_action_txns_writing_the_same_field_data_conflict() { + let dataset = test_dataset(10, 2).await; + assert_conflict(&dataset, tombstone_txn(0, 0), tombstone_txn(0, 0), true).await; + } + + #[tokio::test] + async fn test_removing_a_fragment_conflicts_with_a_concurrent_delete_on_it() { + let dataset = test_dataset(10, 2).await; + let removal = action_txn(vec![TxnAction::RemoveFragment(RemoveFragment { + fragment: ActionRef::Committed(0), + data_change: true, + })]); + let delete = Transaction::new_from_version( + 1, + Operation::Delete { + updated_fragments: vec![dataset.fragments()[0].clone()], + deleted_fragment_ids: vec![], + predicate: "a > 5".into(), + }, + ); + + assert_conflict(&dataset, removal, delete, true).await; + } + + #[tokio::test] + async fn test_an_action_txn_does_not_conflict_with_a_concurrent_append() { + let dataset = test_dataset(10, 2).await; + let append = Transaction::new_from_version( + 1, + Operation::Append { + fragments: vec![{ + let mut fragment = Fragment::new(0); + fragment.physical_rows = Some(5); + fragment + }], + }, + ); + + // An append only mints; it names nothing the action set could touch. + assert_conflict(&dataset, tombstone_txn(0, 0), append, false).await; + } + + /// An update that only moves rows between fragments, which is what a merge + /// insert commits. It translates, so it is compared by footprint rather + /// than conservatively rejected. + fn vertical_update_txn(dataset: &Dataset, fragment: usize) -> Transaction { + Transaction::new_from_version( + 1, + Operation::Update { + removed_fragment_ids: vec![], + updated_fragments: vec![dataset.fragments()[fragment].clone()], + new_fragments: vec![], + fields_modified: vec![], + compacted_sstables: Vec::new(), + fields_for_preserving_frag_bitmap: vec![], + update_mode: None, + inserted_rows_filter: None, + updated_fragment_offsets: None, + }, + ) + } + + #[tokio::test] + async fn test_an_action_txn_is_compared_against_a_concurrent_update_by_footprint() { + let dataset = test_dataset(10, 2).await; + let removal = |fragment| { + action_txn(vec![TxnAction::RemoveFragment(RemoveFragment { + fragment: ActionRef::Committed(fragment), + data_change: true, + })]) + }; + + // The update writes fragment 0's deletion file, so it cannot run + // alongside an action set taking that fragment away -- but it has no + // quarrel with one taking a different fragment away. + assert_conflict(&dataset, removal(0), vertical_update_txn(&dataset, 0), true).await; + assert_conflict( + &dataset, + removal(1), + vertical_update_txn(&dataset, 0), + false, + ) + .await; + } + + #[tokio::test] + async fn test_an_update_does_not_conflict_with_a_column_rewrite_of_the_same_fragment() { + let dataset = test_dataset(10, 2).await; + + // Which rows are gone and what a column holds are separate facts about + // a fragment, so writing a deletion file and rebinding a field's data + // both land. The legacy pairing had to reject this. + assert_conflict( + &dataset, + tombstone_txn(0, 0), + vertical_update_txn(&dataset, 0), + false, + ) + .await; + } + + #[tokio::test] + async fn test_an_update_that_rewrites_rows_stays_conservative() { + let dataset = test_dataset(10, 2).await; + + // Which indices survive a row rewrite is a decision this conversion + // cannot make, so the operation does not translate and the comparison + // falls back to rejecting. + let rewrite = Transaction::new_from_version( + 1, + Operation::Update { + removed_fragment_ids: vec![], + updated_fragments: vec![dataset.fragments()[1].clone()], + new_fragments: vec![], + fields_modified: vec![], + compacted_sstables: Vec::new(), + fields_for_preserving_frag_bitmap: vec![], + update_mode: Some(RewriteRows), + inserted_rows_filter: None, + updated_fragment_offsets: None, + }, + ); + + assert_conflict(&dataset, tombstone_txn(0, 0), rewrite, true).await; + } + + #[tokio::test] + async fn test_an_untranslatable_operation_stays_conservative() { + let dataset = test_dataset(10, 2).await; + let project = Transaction::new_from_version( + 1, + Operation::Project { + schema: dataset.schema().clone(), + preserves_nullability: false, + }, + ); + + assert_conflict(&dataset, tombstone_txn(0, 0), project, true).await; + } + #[tokio::test] async fn test_add_bases_name_conflict() { let dataset = test_dataset(10, 2).await; @@ -4468,7 +4770,8 @@ mod tests { | Operation::UpdateConfig { .. } | Operation::UpdateBases { .. } | Operation::Restore { .. } - | Operation::UpdateMemWalState { .. } => Box::new(std::iter::empty()), + | Operation::UpdateMemWalState { .. } + | Operation::CompositeOperation(_) => Box::new(std::iter::empty()), Operation::Delete { updated_fragments, deleted_fragment_ids,