From 6c1e0aeead5b49309c7c384beb1bacf2f6ef3e63 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Mon, 6 Jul 2026 16:23:02 -0700 Subject: [PATCH] feat(format): general experimental-feature mechanism Proposes and implements a general mechanism for shipping experimental format features without burning a permanent feature-flag bit per experiment. Policy (design doc) and the proto/format change are together so they can be reviewed and voted on as one. Mechanism: - Reserve one persisted bit, FLAG_EXPERIMENTAL (1 << 6, reusing the old first FLAG_UNKNOWN bit), meaning "this dataset uses experimental feature(s)". It is the fail-closed anchor: pre-mechanism libraries reject the dataset on the bit alone. - Carry feature identities in free-form manifest string lists (experimental_reader_features / experimental_writer_features). New libraries admit a dataset iff they understand every declared name. - A compile-time registry (known_experimental_features) lists the experiments a build understands, gated by each experiment's Cargo feature; a default build understands none and so rejects any experimental dataset. This keeps experiments free-flowing (unbounded string namespace, mint/abandon at will) while the bitmap stays conservative (a bit is spent only at graduation). Design, graduation/lifecycle, alternatives, and prior art (Delta Lake table features) in rust/lance-table/design/experimental_feature_flags.md. can_read_dataset / can_write_dataset now take the declared experimental feature list alongside the flags (two internal callers updated). Co-Authored-By: Claude Opus 4.8 (1M context) --- protos/table.proto | 23 +++ .../design/experimental_feature_flags.md | 173 ++++++++++++++++++ rust/lance-table/src/feature_flags.rs | 166 ++++++++++++++--- rust/lance-table/src/format/manifest.rs | 23 +++ rust/lance/src/dataset.rs | 5 +- rust/lance/src/dataset/write/insert.rs | 5 +- 6 files changed, 369 insertions(+), 26 deletions(-) create mode 100644 rust/lance-table/design/experimental_feature_flags.md diff --git a/protos/table.proto b/protos/table.proto index 8d0cb249fda..80803f64efb 100644 --- a/protos/table.proto +++ b/protos/table.proto @@ -115,6 +115,11 @@ message Manifest { // * 1 << 3: table config is present // * 1 << 4: dataset uses multiple base paths // * 1 << 5: transaction file writes are disabled + // * 1 << 6: dataset uses one or more experimental features, named in + // experimental_reader_features / experimental_writer_features. This + // bit is the fail-closed anchor: implementations that predate a + // given experiment reject the dataset on this bit alone, without + // needing to parse the feature names. uint64 reader_feature_flags = 9; // Feature flags for writers. @@ -207,6 +212,24 @@ message Manifest { // The branch of the dataset. None means main branch. optional string branch = 20; + + // Names of experimental features this dataset relies on for reading. + // + // EXPERIMENTAL: this is the forward-looking capability list that pairs with + // the `1 << 6` reader feature-flag bit. A reader that does not recognize + // every name listed here must refuse to read the dataset. Feature names are + // stable, lowercase-kebab identifiers (e.g. "action-transactions"). A name + // is removed from this list once its feature graduates to a dedicated + // feature-flag bit, or when the experiment is abandoned and its data is no + // longer written. Empty for datasets that use no experimental features. + repeated string experimental_reader_features = 22; + + // Names of experimental features this dataset relies on for writing. + // + // EXPERIMENTAL: the writer-side counterpart of experimental_reader_features, + // pairing with the `1 << 6` writer feature-flag bit. A writer that does not + // recognize every name listed here must refuse to commit to the dataset. + repeated string experimental_writer_features = 23; } // Manifest // external dataset base path diff --git a/rust/lance-table/design/experimental_feature_flags.md b/rust/lance-table/design/experimental_feature_flags.md new file mode 100644 index 00000000000..d630ce55fec --- /dev/null +++ b/rust/lance-table/design/experimental_feature_flags.md @@ -0,0 +1,173 @@ +# Proposal: A General Experimental-Feature Mechanism + +Status: **Proposed** (policy + format change; seeking PMC agreement) + +## Summary + +Add a single, general mechanism for shipping *experimental* format features +without spending a scarce, permanent feature-flag bit per experiment: + +- Reserve **one** persisted feature-flag bit, `FLAG_EXPERIMENTAL` (`1 << 6`), + meaning "this dataset uses one or more experimental features." +- Carry the actual feature identities in **free-form string lists** on the + manifest: `experimental_reader_features` and `experimental_writer_features`. +- A build understands an experiment only if its name is in a compile-time + registry (gated by the experiment's Cargo feature). Unknown name → refuse. + +This lets experiments be **free-flowing** (mint/abandon names at will, unbounded +namespace) while the permanent bitmap stays **conservative** (a bit is spent +only when a feature graduates). + +## Motivation + +Feature flags today are two `u64` bitmaps (`reader_feature_flags`, +`writer_feature_flags`) with a linear "first unknown bit" boundary: an +implementation accepts a dataset iff no bit at or above the boundary is set. +This is elegant for *stable* features, but it makes experimentation expensive: + +1. **Every persisted bit is burned forever.** Even an *abandoned* experiment's + bit can never be reused, because some dataset may have written it and reusing + the bit would misread that dataset. Experimental churn permanently pollutes a + scarce (64-bit), shared namespace. +2. **Claiming a bit is high-ceremony**, which chills experimentation — reaching + for a permanent, format-committing bit for a maybe-throwaway idea feels + heavy, so people avoid trying things. + +Only *format-changing* experiments need a persisted flag at all (purely +behavioral experiments are ordinary Cargo features / runtime config and never +touch the bitmap). But for the ones that do, we want a cheap, safe, churnable +path. + +## Design + +### The bit is the fail-closed anchor; the strings are the capability list + +`FLAG_EXPERIMENTAL` reuses bit 64, which was previously the first `FLAG_UNKNOWN` +bit. This is safe to repurpose: until now, writing bit 64 made a dataset +unreadable by *every* implementation, so no dataset in the wild has it set. + +- **Old implementations** (predating any experiment) still treat bit 64 as + unknown and reject the dataset on the bit alone — they never need to parse the + names. This is the whole reason the bit exists. +- **New implementations** ignore the bit for admission and instead check the + string lists directly: a reader admits the dataset iff it understands every + name in `experimental_reader_features` (and likewise for writes). + +The bit is thus **backward-facing** (fail-closed for old readers) and the +strings are **forward-facing** (precise capability negotiation for new readers). + +Why the bit is load-bearing and a string list alone is unsafe: a new manifest +field is, by protobuf's rules, silently ignored by readers that don't know it. +Without the bit, an old reader would skip the experimental-features field and +happily misread experimental data. The bit forces the rejection. + +### Admission algorithm + +```text +can_read(reader_flags, experimental_reader_features): + if any bit >= FLAG_UNKNOWN (128) is set: # a non-experimental unknown + reject + if any name in experimental_reader_features is not in this build's registry: + reject + accept +``` + +`can_write` is identical over the writer flags/list. `FLAG_EXPERIMENTAL` itself +is a *known* bit (below the new `FLAG_UNKNOWN = 128`), so it never trips the +first check; gating is entirely by the name lists. + +### The registry + +Each build exposes the experimental features it understands via a compile-time +list, populated only when the implementing Cargo feature is enabled: + +```rust +pub fn known_experimental_features() -> &'static [&'static str] { + &[ + #[cfg(feature = "unstable-action-transactions")] + "action-transactions", + ] +} +``` + +A default release build understands *none*, so it rejects any dataset that +declares an experimental feature — the desired conservative default. + +### Two independent layers of gating + +- **Cargo feature** (compile-time): keeps experimental *code* out of released + artifacts and out of the support/attack surface. +- **`FLAG_EXPERIMENTAL` + name list** (persisted): keeps experimental *data* + safe across versions and implementations. + +They are orthogonal and both apply. The mechanism itself (bit semantics + name +checking) is **not** Cargo-gated — it must be in every build so that all builds +fail closed on experiments they lack. + +## Graduation and lifecycle + +- **Graduate:** when an experiment is ratified, it gets its own dedicated stable + feature-flag bit. Its name is dropped from `known_experimental_features`, and + new writes stop emitting the name (setting the new stable bit instead). Only + now does the feature consume a permanent bitmap slot — and only because it + earned one. +- **Abandon:** drop the name from the registry. Datasets that used it become + unreadable by all builds (correctly — the feature is gone). No permanent bit + was ever spent. +- **Debt control:** the registry is a single auditable list. An experimental + name that has not graduated within N releases should be removed. Experimental + code lives behind its Cargo feature in isolated modules, not threaded through + stable paths. + +Be **free-flowing in the string list, conservative at the bitmap.** The speed +bump lands at graduation (a real compatibility commitment), not at +experiment-start (where friction just deters trying things). + +## Alternatives considered + +- **A dedicated bit per experiment** (what the action-transactions skeleton PR + #7641 currently does). Simple, but every experiment permanently burns a scarce + bit even if abandoned. Fine for one feature; doesn't scale to a culture of + experimentation. +- **A free-form field with no bit.** Unsafe: old readers silently ignore the + unknown field and misread experimental data. The reserved bit is required as + the fail-closed anchor. +- **A long-lived feature branch instead of flags.** Different tool — it isolates + *code*, not *runtime/format behavior on a shared codebase*. It defers + integration (merge rot, big-bang review), blocks early opt-in dogfooding, and + *still* needs a persisted marker so branch-written data is identifiable and + rejectable. Branches remain the right tool for short throwaway spikes and + compose fine as the per-slice PR mechanism, but not as the isolation model for + a feature. + +## Prior art + +Delta Lake's "table features" made exactly this move: replacing a coarse, +monotonic protocol-version integer with a named, split reader/writer feature set +so that features could be added independently and negotiated by name. This +proposal is the same shape, anchored to Lance's existing feature-flag bitmap for +backward-compatible rejection. + +## Format change + +Two `repeated string` fields on `Manifest` (`table.proto`): +`experimental_reader_features = 22`, `experimental_writer_features = 23`, plus +documentation of bit `1 << 6` on `reader_feature_flags`. Backwards compatible +(new field numbers, new bit that old readers already reject). + +## Impact on #7641 (action-based transactions) + +The action-transactions skeleton (#7641) currently claims a dedicated +`FLAG_ACTION_TRANSACTIONS` bit. Once this mechanism lands, #7641 should adopt it +instead: set `FLAG_EXPERIMENTAL` + list `"action-transactions"`, register that +name under the `unstable-action-transactions` Cargo feature, and free the +dedicated bit. This PR should land first. + +## Open questions + +- Naming convention for feature identifiers (proposed: lowercase-kebab, stable + across the experiment's life). +- Whether to also expose the registry / active experimental features through the + Python and Java bindings for introspection. +- Whether graduation should keep the old experimental name readable (dual-accept + during a transition window) or require a rewrite. diff --git a/rust/lance-table/src/feature_flags.rs b/rust/lance-table/src/feature_flags.rs index 096f0da79e5..42d8d626d4a 100644 --- a/rust/lance-table/src/feature_flags.rs +++ b/rust/lance-table/src/feature_flags.rs @@ -20,8 +20,44 @@ pub const FLAG_TABLE_CONFIG: u64 = 8; pub const FLAG_BASE_PATHS: u64 = 16; /// Disable writing transaction file under _transaction/, this flag is set when we only want to write inline transaction in manifest pub const FLAG_DISABLE_TRANSACTION_FILE: u64 = 32; -/// The first bit that is unknown as a feature flag -pub const FLAG_UNKNOWN: u64 = 64; +/// The dataset relies on one or more experimental features, named in the +/// manifest's `experimental_reader_features` / `experimental_writer_features`. +/// +/// This bit is the fail-closed anchor for the experimental-feature mechanism. +/// It is set whenever the corresponding experimental feature list is non-empty. +/// Libraries built before a given experiment existed treat bit 64 as unknown +/// (it was previously `FLAG_UNKNOWN`) and reject the dataset without needing to +/// parse the feature names. Libraries that understand the mechanism defer to the +/// name lists via [`can_read_dataset`] / [`can_write_dataset`]. See +/// `rust/lance-table/design/experimental_feature_flags.md`. +pub const FLAG_EXPERIMENTAL: u64 = 64; +/// The first bit that is unknown as a feature flag. +pub const FLAG_UNKNOWN: u64 = 128; + +// The experimental bit must be a *known* bit so admission is gated by the name +// lists, not the bitmap boundary. +const _: () = assert!(FLAG_EXPERIMENTAL < FLAG_UNKNOWN); + +/// The experimental feature names this build understands. +/// +/// Each name is registered here only when the Cargo feature that implements it +/// is enabled, so a default build understands none and therefore rejects any +/// dataset that declares an experimental feature. When an experiment graduates, +/// its name moves out of this list and onto a dedicated feature-flag bit. +pub fn known_experimental_features() -> &'static [&'static str] { + &[ + // Register experimental feature names here, gated by their Cargo feature: + // #[cfg(feature = "unstable-action-transactions")] "action-transactions", + ] +} + +/// Whether every name in `experimental_features` is understood by this build. +fn understands_experimental_features(experimental_features: &[String]) -> bool { + let known = known_experimental_features(); + experimental_features + .iter() + .all(|feature| known.contains(&feature.as_str())) +} /// Set the reader and writer feature flags in the manifest based on the contents of the manifest. pub fn apply_feature_flags( @@ -74,15 +110,38 @@ pub fn apply_feature_flags( if disable_transaction_file { manifest.writer_feature_flags |= FLAG_DISABLE_TRANSACTION_FILE; } + + // The experimental bit is the fail-closed anchor for the named experimental + // feature lists; keep it in sync with them. + if !manifest.experimental_reader_features.is_empty() { + manifest.reader_feature_flags |= FLAG_EXPERIMENTAL; + } + if !manifest.experimental_writer_features.is_empty() { + manifest.writer_feature_flags |= FLAG_EXPERIMENTAL; + } Ok(()) } -pub fn can_read_dataset(reader_flags: u64) -> bool { - reader_flags < FLAG_UNKNOWN +/// Whether this build can read a dataset with the given reader feature flags and +/// declared experimental reader features. +/// +/// Rejects if any non-experimental unknown bit is set, or if any declared +/// experimental feature is not understood by this build. +pub fn can_read_dataset(reader_flags: u64, experimental_reader_features: &[String]) -> bool { + if reader_flags & !(FLAG_UNKNOWN - 1) != 0 { + // A bit at or above FLAG_UNKNOWN is set — a feature we don't know about. + return false; + } + understands_experimental_features(experimental_reader_features) } -pub fn can_write_dataset(writer_flags: u64) -> bool { - writer_flags < FLAG_UNKNOWN +/// Whether this build can write to a dataset with the given writer feature flags +/// and declared experimental writer features. See [`can_read_dataset`]. +pub fn can_write_dataset(writer_flags: u64, experimental_writer_features: &[String]) -> bool { + if writer_flags & !(FLAG_UNKNOWN - 1) != 0 { + return false; + } + understands_experimental_features(experimental_writer_features) } pub fn has_deprecated_v2_feature_flag(writer_flags: u64) -> bool { @@ -94,40 +153,99 @@ mod tests { use super::*; use crate::format::BasePath; + const NO_FEATURES: &[String] = &[]; + #[test] fn test_read_check() { - assert!(can_read_dataset(0)); - assert!(can_read_dataset(super::FLAG_DELETION_FILES)); - assert!(can_read_dataset(super::FLAG_STABLE_ROW_IDS)); - assert!(can_read_dataset(super::FLAG_USE_V2_FORMAT_DEPRECATED)); - assert!(can_read_dataset(super::FLAG_TABLE_CONFIG)); - assert!(can_read_dataset(super::FLAG_BASE_PATHS)); - assert!(can_read_dataset(super::FLAG_DISABLE_TRANSACTION_FILE)); + assert!(can_read_dataset(0, NO_FEATURES)); + assert!(can_read_dataset(super::FLAG_DELETION_FILES, NO_FEATURES)); + assert!(can_read_dataset(super::FLAG_STABLE_ROW_IDS, NO_FEATURES)); + assert!(can_read_dataset( + super::FLAG_USE_V2_FORMAT_DEPRECATED, + NO_FEATURES + )); + assert!(can_read_dataset(super::FLAG_TABLE_CONFIG, NO_FEATURES)); + assert!(can_read_dataset(super::FLAG_BASE_PATHS, NO_FEATURES)); + assert!(can_read_dataset( + super::FLAG_DISABLE_TRANSACTION_FILE, + NO_FEATURES + )); assert!(can_read_dataset( super::FLAG_DELETION_FILES | super::FLAG_STABLE_ROW_IDS - | super::FLAG_USE_V2_FORMAT_DEPRECATED + | super::FLAG_USE_V2_FORMAT_DEPRECATED, + NO_FEATURES )); - assert!(!can_read_dataset(super::FLAG_UNKNOWN)); + assert!(!can_read_dataset(super::FLAG_UNKNOWN, NO_FEATURES)); } #[test] fn test_write_check() { - assert!(can_write_dataset(0)); - assert!(can_write_dataset(super::FLAG_DELETION_FILES)); - assert!(can_write_dataset(super::FLAG_STABLE_ROW_IDS)); - assert!(can_write_dataset(super::FLAG_USE_V2_FORMAT_DEPRECATED)); - assert!(can_write_dataset(super::FLAG_TABLE_CONFIG)); - assert!(can_write_dataset(super::FLAG_BASE_PATHS)); - assert!(can_write_dataset(super::FLAG_DISABLE_TRANSACTION_FILE)); + assert!(can_write_dataset(0, NO_FEATURES)); + assert!(can_write_dataset(super::FLAG_DELETION_FILES, NO_FEATURES)); + assert!(can_write_dataset(super::FLAG_STABLE_ROW_IDS, NO_FEATURES)); + assert!(can_write_dataset( + super::FLAG_USE_V2_FORMAT_DEPRECATED, + NO_FEATURES + )); + assert!(can_write_dataset(super::FLAG_TABLE_CONFIG, NO_FEATURES)); + assert!(can_write_dataset(super::FLAG_BASE_PATHS, NO_FEATURES)); + assert!(can_write_dataset( + super::FLAG_DISABLE_TRANSACTION_FILE, + NO_FEATURES + )); assert!(can_write_dataset( super::FLAG_DELETION_FILES | super::FLAG_STABLE_ROW_IDS | super::FLAG_USE_V2_FORMAT_DEPRECATED | super::FLAG_TABLE_CONFIG - | super::FLAG_BASE_PATHS + | super::FLAG_BASE_PATHS, + NO_FEATURES )); - assert!(!can_write_dataset(super::FLAG_UNKNOWN)); + assert!(!can_write_dataset(super::FLAG_UNKNOWN, NO_FEATURES)); + } + + #[test] + fn test_experimental_feature_admission() { + // A default build understands no experimental features, so any declared + // experimental feature is rejected on both read and write paths. + let declared = vec!["some-experiment".to_string()]; + assert!(!can_read_dataset(FLAG_EXPERIMENTAL, &declared)); + assert!(!can_write_dataset(FLAG_EXPERIMENTAL, &declared)); + + // The experimental bit with no declared features is harmless — there is + // nothing the reader could fail to understand. (Pre-mechanism libraries + // still reject bit 64, since their FLAG_UNKNOWN was 64.) + assert!(can_read_dataset(FLAG_EXPERIMENTAL, NO_FEATURES)); + assert!(can_write_dataset(FLAG_EXPERIMENTAL, NO_FEATURES)); + } + + #[test] + fn test_apply_sets_experimental_flag() { + use crate::format::{DataStorageFormat, Manifest}; + use arrow_schema::{Field as ArrowField, Schema as ArrowSchema}; + use lance_core::datatypes::Schema; + use std::collections::HashMap; + use std::sync::Arc; + + let arrow_schema = ArrowSchema::new(vec![ArrowField::new( + "x", + arrow_schema::DataType::Int64, + false, + )]); + let schema = Schema::try_from(&arrow_schema).unwrap(); + let mut manifest = Manifest::new( + schema, + Arc::new(vec![]), + DataStorageFormat::default(), + HashMap::new(), + ); + manifest.experimental_writer_features = vec!["some-experiment".to_string()]; + + apply_feature_flags(&mut manifest, false, false).unwrap(); + + assert_ne!(manifest.writer_feature_flags & FLAG_EXPERIMENTAL, 0); + assert_eq!(manifest.reader_feature_flags & FLAG_EXPERIMENTAL, 0); } #[test] diff --git a/rust/lance-table/src/format/manifest.rs b/rust/lance-table/src/format/manifest.rs index 9845061b7e4..da864ca06a9 100644 --- a/rust/lance-table/src/format/manifest.rs +++ b/rust/lance-table/src/format/manifest.rs @@ -101,6 +101,19 @@ pub struct Manifest { /* external base paths */ pub base_paths: HashMap, + + /// Names of experimental features this dataset relies on for reading. + /// + /// EXPERIMENTAL: pairs with the `FLAG_EXPERIMENTAL` reader feature-flag bit. + /// A reader that does not understand every name here must refuse the dataset. + /// Empty for datasets that use no experimental features. + pub experimental_reader_features: Vec, + + /// Names of experimental features this dataset relies on for writing. + /// + /// EXPERIMENTAL: the writer-side counterpart of + /// `experimental_reader_features`. + pub experimental_writer_features: Vec, } // We use the most significant bit to indicate that a transaction is detached @@ -196,6 +209,8 @@ impl Manifest { config: HashMap::new(), table_metadata: HashMap::new(), base_paths, + experimental_reader_features: Vec::new(), + experimental_writer_features: Vec::new(), } } @@ -227,6 +242,8 @@ impl Manifest { config: previous.config.clone(), table_metadata: previous.table_metadata.clone(), base_paths: previous.base_paths.clone(), + experimental_reader_features: previous.experimental_reader_features.clone(), + experimental_writer_features: previous.experimental_writer_features.clone(), } } @@ -289,6 +306,8 @@ impl Manifest { base_paths }, table_metadata: self.table_metadata.clone(), + experimental_reader_features: self.experimental_reader_features.clone(), + experimental_writer_features: self.experimental_writer_features.clone(), } } @@ -931,6 +950,8 @@ impl TryFrom for Manifest { .iter() .map(|item| (item.id, item.clone().into())) .collect(), + experimental_reader_features: p.experimental_reader_features, + experimental_writer_features: p.experimental_writer_features, }) } } @@ -994,6 +1015,8 @@ impl From<&Manifest> for pb::Manifest { }) .collect(), transaction_section: m.transaction_section.map(|i| i as u64), + experimental_reader_features: m.experimental_reader_features.clone(), + experimental_writer_features: m.experimental_writer_features.clone(), } } } diff --git a/rust/lance/src/dataset.rs b/rust/lance/src/dataset.rs index 470c6873dc7..234d5b31d18 100644 --- a/rust/lance/src/dataset.rs +++ b/rust/lance/src/dataset.rs @@ -695,7 +695,10 @@ impl Dataset { read_struct(object_reader.as_ref(), offset).await }?; - if !can_read_dataset(manifest.reader_feature_flags) { + if !can_read_dataset( + manifest.reader_feature_flags, + &manifest.experimental_reader_features, + ) { let message = format!( "This dataset cannot be read by this version of Lance. \ Please upgrade Lance to read this dataset.\n Flags: {}", diff --git a/rust/lance/src/dataset/write/insert.rs b/rust/lance/src/dataset/write/insert.rs index 6e1db342f9c..345dfcfa658 100644 --- a/rust/lance/src/dataset/write/insert.rs +++ b/rust/lance/src/dataset/write/insert.rs @@ -343,7 +343,10 @@ impl<'a> InsertBuilder<'a> { // Feature flags if let WriteDestination::Dataset(dataset) = &context.dest - && !can_write_dataset(dataset.manifest.writer_feature_flags) + && !can_write_dataset( + dataset.manifest.writer_feature_flags, + &dataset.manifest.experimental_writer_features, + ) { let message = format!( "This dataset cannot be written by this version of Lance. \