refactor: centralize dataset version policies

rust/lance-file/src/version.rs

@@ -6,8 +6,6 @@ use std::{
     str::FromStr,
 };
 
-use lance_arrow::DataTypeExt;
-use lance_core::datatypes::Field;
 use lance_core::deepsize::{Context, DeepSizeOf};
 use lance_core::{Error, Result};
 
@@ -17,11 +15,21 @@ pub const V2_FORMAT_2_1: &str = "2.1";
 pub const V2_FORMAT_2_2: &str = "2.2";
 pub const V2_FORMAT_2_3: &str = "2.3";
 
+/// Resolve the current stable release policy to an exact file version.
+pub const fn stable_file_version() -> ConcreteFileVersion {
+    ConcreteFileVersion::V2_1
+}
+
+/// Resolve the current next release policy to an exact file version.
+pub const fn next_file_version() -> ConcreteFileVersion {
+    ConcreteFileVersion::V2_3
+}
+
 /// A caller-facing Lance file-version request.
 ///
-/// This selector remains separate from [`ConcreteFileVersion`] because `Stable`
-/// and `Next` are release policy rather than persisted identities.
-#[derive(Debug, Default, PartialEq, Eq, Clone, Copy, Ord, PartialOrd, Hash)]
+/// `Stable` and `Next` are release selectors. They resolve to an exact
+/// [`ConcreteFileVersion`] before file or dataset dispatch and are never persisted.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum LanceFileVersion {
     /// The legacy v1 format.
     Legacy,
@@ -47,42 +55,22 @@ impl DeepSizeOf for LanceFileVersion {
 }
 
 impl LanceFileVersion {
-    /// Resolve release selectors to their current exact selector.
-    pub fn resolve(&self) -> Self {
+    /// Resolve this request through the current release policy.
+    pub const fn resolve(self) -> ConcreteFileVersion {
         match self {
-            Self::Stable => Self::default(),
-            Self::Next => Self::V2_3,
-            _ => *self,
+            Self::Legacy => ConcreteFileVersion::V1,
+            Self::V2_0 => ConcreteFileVersion::V2_0,
+            Self::V2_1 => ConcreteFileVersion::V2_1,
+            Self::Stable => stable_file_version(),
+            Self::V2_2 => ConcreteFileVersion::V2_2,
+            Self::Next => next_file_version(),
+            Self::V2_3 => ConcreteFileVersion::V2_3,
         }
     }
 
-    pub fn is_unstable(&self) -> bool {
-        self >= &Self::Next
-    }
-
-    pub fn try_from_major_minor(major: u32, minor: u32) -> Result<Self> {
-        ConcreteFileVersion::from_data_file_numbers(major, minor).map(Into::into)
-    }
-
-    pub fn to_numbers(&self) -> (u32, u32) {
-        let (major, minor) = ConcreteFileVersion::from(*self).to_data_file_numbers();
-        (major, minor)
-    }
-
-    pub fn iter_non_legacy() -> impl Iterator<Item = Self> {
-        [Self::V2_0, Self::V2_1, Self::V2_2, Self::V2_3].into_iter()
-    }
-
-    pub fn support_add_sub_column(&self) -> bool {
-        self > &Self::V2_1
-    }
-
-    pub fn support_remove_sub_column(&self, field: &Field) -> bool {
-        if self <= &Self::V2_1 {
-            field.data_type().is_struct()
-        } else {
-            field.data_type().is_nested()
-        }
+    /// Whether this request resolves to an unstable exact format.
+    pub const fn is_unstable(self) -> bool {
+        self.resolve().is_unstable()
     }
 }
 
@@ -143,6 +131,24 @@ impl DeepSizeOf for ConcreteFileVersion {
 }
 
 impl ConcreteFileVersion {
+    /// Convert this exact identity to the corresponding exact public selector.
+    ///
+    /// This never produces the release selectors `stable` or `next`.
+    pub const fn to_selector(self) -> LanceFileVersion {
+        match self {
+            Self::V1 => LanceFileVersion::Legacy,
+            Self::V2_0 => LanceFileVersion::V2_0,
+            Self::V2_1 => LanceFileVersion::V2_1,
+            Self::V2_2 => LanceFileVersion::V2_2,
+            Self::V2_3 => LanceFileVersion::V2_3,
+        }
+    }
+
+    /// Whether this exact format is covered only by the unstable release policy.
+    pub const fn is_unstable(self) -> bool {
+        matches!(self, Self::V2_3)
+    }
+
     /// Decode the exact version string stored in a dataset manifest.
     ///
     /// Public selector aliases such as `legacy`, `0.3`, `stable`, and `next` are
@@ -241,30 +247,15 @@ impl Display for ConcreteFileVersion {
     }
 }
 
-impl From<ConcreteFileVersion> for LanceFileVersion {
-    fn from(value: ConcreteFileVersion) -> Self {
-        match value {
-            ConcreteFileVersion::V1 => Self::Legacy,
-            ConcreteFileVersion::V2_0 => Self::V2_0,
-            ConcreteFileVersion::V2_1 => Self::V2_1,
-            ConcreteFileVersion::V2_2 => Self::V2_2,
-            ConcreteFileVersion::V2_3 => Self::V2_3,
-        }
+impl From<LanceFileVersion> for ConcreteFileVersion {
+    fn from(value: LanceFileVersion) -> Self {
+        value.resolve()
     }
 }
 
-impl From<LanceFileVersion> for ConcreteFileVersion {
-    fn from(value: LanceFileVersion) -> Self {
-        match value.resolve() {
-            LanceFileVersion::Legacy => Self::V1,
-            LanceFileVersion::V2_0 => Self::V2_0,
-            LanceFileVersion::V2_1 => Self::V2_1,
-            LanceFileVersion::V2_2 => Self::V2_2,
-            LanceFileVersion::V2_3 => Self::V2_3,
-            LanceFileVersion::Stable | LanceFileVersion::Next => {
-                unreachable!("resolved file-version selector must be exact")
-            }
-        }
+impl From<ConcreteFileVersion> for LanceFileVersion {
+    fn from(value: ConcreteFileVersion) -> Self {
+        value.to_selector()
     }
 }
 
@@ -302,8 +293,7 @@ mod tests {
         ];
 
         for (selector, expected) in cases {
-            assert_eq!(ConcreteFileVersion::from(selector), expected);
-            assert_eq!(LanceFileVersion::from(expected), selector.resolve());
+            assert_eq!(selector.resolve(), expected);
         }
     }
 

rust/lance-table/src/format/fragment.rs

@@ -168,7 +168,7 @@ impl DataFile {
         full_schema.project_by_ids(&self.fields, false)
     }
 
-    pub fn is_legacy_file(&self) -> bool {
+    fn uses_v1_data_file_encoding(&self) -> bool {
         self.file_major_version == 0 && self.file_minor_version < 3
     }
 
@@ -181,7 +181,7 @@ impl DataFile {
     }
 
     pub fn validate(&self, base_path: &Path) -> Result<()> {
-        if self.is_legacy_file() {
+        if self.uses_v1_data_file_encoding() {
             if !self.fields.windows(2).all(|w| w[0] < w[1]) {
                 return Err(Error::corrupt_file(
                     base_path.clone().join(self.path.clone()),
@@ -620,12 +620,6 @@ impl Fragment {
             .push(DataFile::new_legacy(path, schema, None, None));
     }
 
-    // True if this fragment is made up of legacy v1 files, false otherwise
-    pub fn has_legacy_files(&self) -> bool {
-        // If any file in a fragment is legacy then all files in the fragment must be
-        self.files[0].is_legacy_file()
-    }
-
     // Helper method to infer the Lance version from a set of fragments
     //
     // Returns None if there are no data files

rust/lance-table/src/format/manifest.rs

@@ -5,10 +5,9 @@ use async_trait::async_trait;
 use chrono::prelude::*;
 use lance_core::deepsize::DeepSizeOf;
 use lance_file::datatypes::{Fields, FieldsWithMeta};
-use lance_file::version::{ConcreteFileVersion, LanceFileVersion};
-use lance_file::versions::v1::{
-    encoding::populate_schema_dictionaries, reader::FileReader as V1FileReader,
-};
+use lance_file::version::{ConcreteFileVersion, stable_file_version};
+use lance_file::versions::v1::encoding::populate_schema_dictionaries;
+use lance_file::versions::v1::reader::FileReader as V1FileReader;
 use lance_io::traits::{ProtoStruct, Reader};
 use object_store::path::Path;
 use prost::Message;
@@ -503,10 +502,6 @@ impl Manifest {
         pb_manifest.encode_to_vec()
     }
 
-    pub fn should_use_legacy_format(&self) -> bool {
-        self.data_storage_format.version == ConcreteFileVersion::V1
-    }
-
     /// Get the summary information of a manifest.
     ///
     /// This function calculates various statistics about the manifest, including:
@@ -555,6 +550,10 @@ impl Manifest {
 }
 
 /// Populate dictionary values stored outside a v1 manifest.
+///
+/// Current manifests carry dictionary values in the schema and require no work.
+/// Keeping this decision here prevents dataset readers from reconstructing
+/// manifest-format behavior from the data-file version.
 pub async fn populate_manifest_schema_dictionary(
     manifest: &mut Manifest,
     reader: &dyn Reader,
@@ -642,16 +641,11 @@ impl DataStorageFormat {
     pub fn lance_file_format(&self) -> ConcreteFileVersion {
         self.version
     }
-
-    // Retained until all selector-based execution APIs migrate to exact versions.
-    pub fn lance_file_version(&self) -> Result<LanceFileVersion> {
-        Ok(self.version.into())
-    }
 }
 
 impl Default for DataStorageFormat {
     fn default() -> Self {
-        Self::new(ConcreteFileVersion::from(LanceFileVersion::Stable))
+        Self::new(stable_file_version())
     }
 }
 
@@ -918,7 +912,7 @@ impl TryFrom<pb::Manifest> for Manifest {
                 } else {
                     // No fragments to inspect, best we can do is look at writer flags
                     if has_deprecated_v2_feature_flag(p.writer_feature_flags) {
-                        DataStorageFormat::new(ConcreteFileVersion::from(LanceFileVersion::Stable))
+                        DataStorageFormat::new(stable_file_version())
                     } else {
                         DataStorageFormat::new(ConcreteFileVersion::V1)
                     }
@@ -1117,7 +1111,7 @@ mod tests {
         .unwrap();
         assert_eq!(
             recovered_stable.data_storage_format.lance_file_format(),
-            ConcreteFileVersion::from(LanceFileVersion::Stable)
+            stable_file_version()
         );
     }
 
@@ -1581,7 +1575,7 @@ mod tests {
                 "data_with_deletion.lance",
                 vec![0, 1],
                 vec![0, 1],
-                ConcreteFileVersion::from(LanceFileVersion::Stable),
+                stable_file_version(),
                 NonZero::new(1000),
             )
             .with_physical_rows(50);

rust/lance-table/src/io/commit.rs

@@ -2101,7 +2101,7 @@ mod tests {
 
         use arrow_schema::{DataType, Field as ArrowField, Schema as ArrowSchema};
         use lance_core::datatypes::Schema;
-        use lance_file::version::{ConcreteFileVersion, LanceFileVersion};
+        use lance_file::version::LanceFileVersion;
 
         use crate::format::DataStorageFormat;
 
@@ -2116,7 +2116,7 @@ mod tests {
         let mut manifest = Manifest::new(
             Schema::try_from(&arrow_schema).unwrap(),
             Arc::new(vec![]),
-            DataStorageFormat::new(ConcreteFileVersion::from(LanceFileVersion::Stable)),
+            DataStorageFormat::new(LanceFileVersion::Stable.resolve()),
             HashMap::new(),
         );
 
@@ -2158,7 +2158,7 @@ mod tests {
 
         use arrow_schema::{DataType, Field as ArrowField, Schema as ArrowSchema};
         use lance_core::datatypes::Schema;
-        use lance_file::version::{ConcreteFileVersion, LanceFileVersion};
+        use lance_file::version::LanceFileVersion;
 
         use crate::format::DataStorageFormat;
 
@@ -2175,7 +2175,7 @@ mod tests {
         let mut manifest = Manifest::new(
             Schema::try_from(&arrow_schema).unwrap(),
             Arc::new(vec![]),
-            DataStorageFormat::new(ConcreteFileVersion::from(LanceFileVersion::Stable)),
+            DataStorageFormat::new(LanceFileVersion::Stable.resolve()),
             HashMap::new(),
         );