refactor: Clean up table feature parsing with strum-0.28

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"] }

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, IntoTableFeature, 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;
@@ -426,17 +425,17 @@ 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<impl IntoIterator<Item = impl IntoTableFeature>>,
+    features: Option<impl IntoIterator<Item = impl Into<TableFeature>>>,
 ) -> Option<Vec<TableFeature>> {
-    let features = features?.into_iter().map(|f| f.into_table_feature());
+    let features = features?.into_iter().map(Into::into);
     Some(features.collect())
 }
 
 impl Protocol {
     /// Try to create a new modern Protocol instance with the given table feature lists
     pub(crate) fn try_new_modern(
-        reader_features: impl IntoIterator<Item = impl IntoTableFeature>,
-        writer_features: impl IntoIterator<Item = impl IntoTableFeature>,
+        reader_features: impl IntoIterator<Item = impl Into<TableFeature>>,
+        writer_features: impl IntoIterator<Item = impl Into<TableFeature>>,
     ) -> DeltaResult<Self> {
         Self::try_new(
             TABLE_FEATURES_MIN_READER_VERSION,
@@ -464,8 +463,8 @@ impl Protocol {
     pub(crate) fn try_new(
         min_reader_version: i32,
         min_writer_version: i32,
-        reader_features: Option<impl IntoIterator<Item = impl IntoTableFeature>>,
-        writer_features: Option<impl IntoIterator<Item = impl IntoTableFeature>>,
+        reader_features: Option<impl IntoIterator<Item = impl Into<TableFeature>>>,
+        writer_features: Option<impl IntoIterator<Item = impl Into<TableFeature>>>,
     ) -> DeltaResult<Self> {
         let reader_features = parse_features(reader_features);
         let writer_features = parse_features(writer_features);

kernel/src/table_features/mod.rs

@@ -65,11 +65,7 @@ pub const SET_TABLE_FEATURE_SUPPORTED_VALUE: &str = "supported";
     EnumCount,
     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
-)]
+#[strum(serialize_all = "camelCase")]
 #[serde(rename_all = "camelCase")]
 #[internal_api]
 #[derive(EnumIter)]
@@ -726,38 +722,15 @@ impl TableFeature {
     }
 }
 
-/// Like `Into<TableFeature>`, 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<String> for TableFeature {
+    fn from(feature: String) -> Self {
+        feature.as_str().into()
     }
 }
 
@@ -841,7 +814,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();