From 6486c116e80bbb980c301054d9a32efc12277037 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 23 Feb 2026 09:58:56 -0800 Subject: [PATCH 1/2] refactor: Clean up table feature parsing with strum-0.28 --- kernel/Cargo.toml | 2 +- kernel/src/actions/mod.rs | 10 ++++---- kernel/src/table_features/mod.rs | 41 +++++++------------------------- 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 1e04d920e4..92483df3f2 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -48,7 +48,7 @@ itertools = "0.14" roaring = "0.11.2" serde = { version = "1", features = ["derive", "rc"] } serde_json = "1" -strum = { version = "0.27", features = ["derive"] } +strum = { version = "0.28", features = ["derive"] } thiserror = "2" # only for structured logging tracing = { version = "0.1", features = ["log"] } diff --git a/kernel/src/actions/mod.rs b/kernel/src/actions/mod.rs index 27a5fe8ac6..b860d96196 100644 --- a/kernel/src/actions/mod.rs +++ b/kernel/src/actions/mod.rs @@ -7,7 +7,7 @@ use std::sync::{Arc, LazyLock}; use self::deletion_vector::DeletionVectorDescriptor; use crate::expressions::{MapData, Scalar, StructData}; use crate::schema::{DataType, MapType, SchemaRef, StructField, StructType, ToSchema as _}; -use crate::table_features::{FeatureType, IntoTableFeature, TableFeature}; +use crate::table_features::{FeatureType, TableFeature}; use crate::table_properties::TableProperties; use crate::utils::require; use crate::{ @@ -423,9 +423,9 @@ pub(crate) struct Protocol { /// Parse a list of feature identifiers into TableFeatures. Returns `None` for `None` input; /// otherwise infallible (unrecognized names become `TableFeature::Unknown`). fn parse_features( - features: Option>, + features: Option>>, ) -> Option> { - let features = features?.into_iter().map(|f| f.into_table_feature()); + let features = features?.into_iter().map(Into::into); Some(features.collect()) } @@ -456,8 +456,8 @@ impl Protocol { pub(crate) fn try_new( min_reader_version: i32, min_writer_version: i32, - reader_features: Option>, - writer_features: Option>, + reader_features: Option>>, + writer_features: Option>>, ) -> DeltaResult { let reader_features = parse_features(reader_features); let writer_features = parse_features(writer_features); diff --git a/kernel/src/table_features/mod.rs b/kernel/src/table_features/mod.rs index 53953de865..5790a3790b 100644 --- a/kernel/src/table_features/mod.rs +++ b/kernel/src/table_features/mod.rs @@ -60,9 +60,7 @@ pub const SET_TABLE_FEATURE_SUPPORTED_VALUE: &str = "supported"; Hash, )] #[strum( - serialize_all = "camelCase", - parse_err_fn = xxx__not_needed__default_variant_means_parsing_is_infallible__xxx, - parse_err_ty = Infallible // ignored, sadly: https://github.com/Peternator7/strum/issues/430 + serialize_all = "camelCase" )] #[serde(rename_all = "camelCase")] #[internal_api] @@ -720,38 +718,15 @@ impl TableFeature { } } -/// Like `Into`, but avoids collisions between strum's derived `EnumString` and the -/// blanket impl `TryFrom<&str>` that `From<&str> for TableFeature` would trigger. -/// -/// Parsing is infallible: the `Unknown` default variant catches any unrecognized feature name. If -/// https://github.com/Peternator7/strum/pull/432 merges, use impl From for TableFeature instead. -pub(crate) trait IntoTableFeature { - fn into_table_feature(self) -> TableFeature; -} - -impl IntoTableFeature for TableFeature { - fn into_table_feature(self) -> TableFeature { - self - } -} - -impl IntoTableFeature for &TableFeature { - fn into_table_feature(self) -> TableFeature { - self.clone() - } -} - -/// Parsing is infallible thanks to `TableFeature::Unknown` default variant -impl IntoTableFeature for &str { - fn into_table_feature(self) -> TableFeature { - #[allow(clippy::unwrap_used)] // infallible, see strum parse_err_fn - self.parse().unwrap() +impl From<&TableFeature> for TableFeature { + fn from(feature: &TableFeature) -> Self { + feature.clone() } } -impl IntoTableFeature for String { - fn into_table_feature(self) -> TableFeature { - self.as_str().into_table_feature() +impl From for TableFeature { + fn from(feature: String) -> Self { + feature.as_str().into() } } @@ -835,7 +810,7 @@ mod tests { // strum assert_eq!(feature.to_string(), expected); - assert_eq!(feature, expected.into_table_feature()); + assert_eq!(feature, expected.into()); // json let serialized = serde_json::to_string(&feature).unwrap(); From 7e427487eaaeae2cb0d289dad160d0ff35771c81 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 23 Feb 2026 10:26:32 -0800 Subject: [PATCH 2/2] fmt --- kernel/src/actions/mod.rs | 3 +-- kernel/src/table_features/mod.rs | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/src/actions/mod.rs b/kernel/src/actions/mod.rs index af9ff7008b..bd180cf6d7 100644 --- a/kernel/src/actions/mod.rs +++ b/kernel/src/actions/mod.rs @@ -8,8 +8,7 @@ use self::deletion_vector::DeletionVectorDescriptor; use crate::expressions::{MapData, Scalar, StructData}; use crate::schema::{DataType, MapType, SchemaRef, StructField, StructType, ToSchema as _}; use crate::table_features::{ - FeatureType, TableFeature, TABLE_FEATURES_MIN_READER_VERSION, - TABLE_FEATURES_MIN_WRITER_VERSION, + FeatureType, TableFeature, TABLE_FEATURES_MIN_READER_VERSION, TABLE_FEATURES_MIN_WRITER_VERSION, }; use crate::table_properties::TableProperties; use crate::utils::require; diff --git a/kernel/src/table_features/mod.rs b/kernel/src/table_features/mod.rs index 0348525b01..f063617a17 100644 --- a/kernel/src/table_features/mod.rs +++ b/kernel/src/table_features/mod.rs @@ -65,9 +65,7 @@ pub const SET_TABLE_FEATURE_SUPPORTED_VALUE: &str = "supported"; EnumCount, Hash, )] -#[strum( - serialize_all = "camelCase" -)] +#[strum(serialize_all = "camelCase")] #[serde(rename_all = "camelCase")] #[internal_api] #[derive(EnumIter)]