Add enumset to refactor ensure_supported_features

Author: maruschin

kernel/Cargo.toml

@@ -87,6 +87,7 @@ tokio = { version = "1.40", optional = true, features = ["rt-multi-thread"] }
 # Used in integration tests
 hdfs-native = { workspace = true, optional = true }
 walkdir = { workspace = true, optional = true }
+enumset = "1.1.5"
 
 [features]
 # The default version to be expected

kernel/src/actions/mod.rs

@@ -1,18 +1,16 @@
 //! Provides parsing and manipulation of the various actions defined in the [Delta
 //! specification](https://github.com/delta-io/delta/blob/master/PROTOCOL.md)
 
-use std::any::type_name;
-use std::collections::{HashMap, HashSet};
-use std::fmt::{Debug, Display};
-use std::hash::Hash;
-use std::str::FromStr;
+use std::collections::HashMap;
+use std::fmt::Debug;
 use std::sync::LazyLock;
 
 use self::deletion_vector::DeletionVectorDescriptor;
 use crate::actions::schemas::GetStructField;
 use crate::schema::{SchemaRef, StructType};
 use crate::table_features::{
-    ReaderFeature, WriterFeature, SUPPORTED_READER_FEATURES, SUPPORTED_WRITER_FEATURES,
+    ensure_supported_features, ReaderFeature, WriterFeature, SUPPORTED_READER_FEATURES,
+    SUPPORTED_WRITER_FEATURES,
 };
 use crate::table_properties::TableProperties;
 use crate::utils::require;
@@ -321,44 +319,6 @@ impl Protocol {
     }
 }
 
-// given unparsed `table_features`, parse and check if they are subset of `supported_features`
-pub(crate) fn ensure_supported_features<T>(
-    table_features: &[String],
-    supported_features: &HashSet<T>,
-) -> DeltaResult<()>
-where
-    <T as FromStr>::Err: Display,
-    T: Debug + FromStr + Hash + Eq,
-{
-    let error = |unsupported, unsupported_or_unknown| {
-        let supported = supported_features.iter().collect::<Vec<_>>();
-        let features_type = type_name::<T>()
-            .rsplit("::")
-            .next()
-            .unwrap_or("table features");
-        Error::Unsupported(format!(
-            "{} {} {:?}. Supported {} are {:?}",
-            unsupported_or_unknown, features_type, unsupported, features_type, supported
-        ))
-    };
-    let parsed_features: HashSet<T> = table_features
-        .iter()
-        .map(|s| T::from_str(s).map_err(|_| error(vec![s.to_string()], "Unknown")))
-        .collect::<Result<_, Error>>()?;
-
-    // check that parsed features are a subset of supported features
-    parsed_features
-        .is_subset(supported_features)
-        .then_some(())
-        .ok_or_else(|| {
-            let unsupported = parsed_features
-                .difference(supported_features)
-                .map(|f| format!("{:?}", f))
-                .collect::<Vec<_>>();
-            error(unsupported, "Unsupported")
-        })
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Schema)]
 #[cfg_attr(feature = "developer-visibility", visibility::make(pub))]
 #[cfg_attr(test, derive(Serialize, Default), serde(rename_all = "camelCase"))]

kernel/src/table_changes/mod.rs

@@ -31,18 +31,19 @@
 //! let table_change_batches = table_changes_scan.execute(engine.clone())?;
 //! # Ok::<(), Error>(())
 //! ```
-use std::collections::HashSet;
 use std::sync::{Arc, LazyLock};
 
 use scan::TableChangesScanBuilder;
 use url::Url;
 
-use crate::actions::{ensure_supported_features, Protocol};
+use crate::actions::Protocol;
 use crate::log_segment::LogSegment;
 use crate::path::AsUrl;
 use crate::schema::{DataType, Schema, StructField, StructType};
 use crate::snapshot::Snapshot;
-use crate::table_features::{ColumnMappingMode, ReaderFeature};
+use crate::table_features::{
+    ensure_supported_features, ColumnMappingMode, CDF_SUPPORTED_READER_FEATURES,
+};
 use crate::table_properties::TableProperties;
 use crate::utils::require;
 use crate::{DeltaResult, Engine, Error, Version};
@@ -252,8 +253,6 @@ fn check_cdf_table_properties(table_properties: &TableProperties) -> DeltaResult
 /// Ensures that Change Data Feed is supported for a table with this [`Protocol`] .
 /// See the documentation of [`TableChanges`] for more details.
 fn ensure_cdf_read_supported(protocol: &Protocol) -> DeltaResult<()> {
-    static CDF_SUPPORTED_READER_FEATURES: LazyLock<HashSet<ReaderFeature>> =
-        LazyLock::new(|| HashSet::from([ReaderFeature::DeletionVectors]));
     match &protocol.reader_features() {
         // if min_reader_version = 3 and all reader features are subset of supported => OK
         Some(reader_features) if protocol.min_reader_version() == 3 => {

kernel/src/table_configuration.rs

@@ -8,16 +8,14 @@
 //! [`TableProperties`].
 //!
 //! [`Schema`]: crate::schema::Schema
-use std::collections::HashSet;
-use std::sync::{Arc, LazyLock};
-
+use std::sync::Arc;
 use url::Url;
 
-use crate::actions::{ensure_supported_features, Metadata, Protocol};
+use crate::actions::{Metadata, Protocol};
 use crate::schema::{InvariantChecker, SchemaRef};
 use crate::table_features::{
-    column_mapping_mode, validate_schema_column_mapping, ColumnMappingMode, ReaderFeature,
-    WriterFeature,
+    column_mapping_mode, ensure_supported_features, validate_schema_column_mapping,
+    ColumnMappingMode, ReaderFeature, WriterFeature, CDF_SUPPORTED_READER_FEATURES,
 };
 use crate::table_properties::TableProperties;
 use crate::{DeltaResult, Error, Version};
@@ -156,8 +154,6 @@ impl TableConfiguration {
     /// [`TableChanges`]: crate::table_changes::TableChanges
     #[cfg_attr(feature = "developer-visibility", visibility::make(pub))]
     pub(crate) fn is_cdf_read_supported(&self) -> bool {
-        static CDF_SUPPORTED_READER_FEATURES: LazyLock<HashSet<ReaderFeature>> =
-            LazyLock::new(|| HashSet::from([ReaderFeature::DeletionVectors]));
         let protocol_supported = match self.protocol.reader_features() {
             // if min_reader_version = 3 and all reader features are subset of supported => OK
             Some(reader_features) if self.protocol.min_reader_version() == 3 => {

kernel/src/table_features/mod.rs

@@ -1,7 +1,7 @@
-use std::collections::HashSet;
-use std::sync::LazyLock;
-
-use serde::{Deserialize, Serialize};
+use crate::{DeltaResult, Error};
+use enumset::{enum_set, EnumSet, EnumSetType};
+use serde::{de::DeserializeOwned, Deserialize, Serialize};
+use std::fmt::Display;
 use strum::{AsRefStr, Display as StrumDisplay, EnumString, VariantNames};
 
 pub(crate) use column_mapping::column_mapping_mode;
@@ -18,14 +18,12 @@ mod column_mapping;
     Serialize,
     Deserialize,
     Debug,
-    Clone,
-    Eq,
-    PartialEq,
     EnumString,
     StrumDisplay,
     AsRefStr,
     VariantNames,
     Hash,
+    EnumSetType,
 )]
 #[strum(serialize_all = "camelCase")]
 #[serde(rename_all = "camelCase")]
@@ -59,14 +57,12 @@ pub enum ReaderFeature {
     Serialize,
     Deserialize,
     Debug,
-    Clone,
-    Eq,
-    PartialEq,
     EnumString,
     StrumDisplay,
     AsRefStr,
     VariantNames,
     Hash,
+    EnumSetType,
 )]
 #[strum(serialize_all = "camelCase")]
 #[serde(rename_all = "camelCase")]
@@ -123,30 +119,51 @@ impl From<WriterFeature> for String {
     }
 }
 
-pub(crate) static SUPPORTED_READER_FEATURES: LazyLock<HashSet<ReaderFeature>> =
-    LazyLock::new(|| {
-        HashSet::from([
-            ReaderFeature::ColumnMapping,
-            ReaderFeature::DeletionVectors,
-            ReaderFeature::TimestampWithoutTimezone,
-            ReaderFeature::TypeWidening,
-            ReaderFeature::TypeWideningPreview,
-            ReaderFeature::VacuumProtocolCheck,
-            ReaderFeature::V2Checkpoint,
-        ])
-    });
-
-pub(crate) static SUPPORTED_WRITER_FEATURES: LazyLock<HashSet<WriterFeature>> =
+pub(crate) static SUPPORTED_READER_FEATURES: EnumSet<ReaderFeature> = enum_set!(
+    ReaderFeature::ColumnMapping
+        | ReaderFeature::DeletionVectors
+        | ReaderFeature::TimestampWithoutTimezone
+        | ReaderFeature::TypeWidening
+        | ReaderFeature::TypeWideningPreview
+        | ReaderFeature::VacuumProtocolCheck
+        | ReaderFeature::V2Checkpoint
+);
+
+pub(crate) static SUPPORTED_WRITER_FEATURES: EnumSet<WriterFeature> =
     // note: we 'support' Invariants, but only insofar as we check that they are not present.
     // we support writing to tables that have Invariants enabled but not used. similarly, we only
     // support DeletionVectors in that we never write them (no DML).
-    LazyLock::new(|| {
-            HashSet::from([
-                WriterFeature::AppendOnly,
-                WriterFeature::DeletionVectors,
-                WriterFeature::Invariants,
-            ])
-        });
+    enum_set!(
+        WriterFeature::AppendOnly | WriterFeature::DeletionVectors | WriterFeature::Invariants
+    );
+
+pub(crate) static CDF_SUPPORTED_READER_FEATURES: EnumSet<ReaderFeature> =
+    enum_set!(ReaderFeature::DeletionVectors);
+
+pub(crate) fn ensure_supported_features<F>(
+    features: &[String],
+    supported: &EnumSet<F>,
+) -> DeltaResult<()>
+where
+    F: DeserializeOwned + EnumSetType + Display,
+{
+    for feature_str in features {
+        match serde_json::from_str::<F>(feature_str) {
+            Ok(feature_enum) => {
+                if !supported.contains(feature_enum) {
+                    return Err(Error::Unsupported(format!(
+                        "Unsupported feature: {}",
+                        feature_enum
+                    )));
+                }
+            }
+            Err(err) => {
+                return Err(Error::Unsupported(format!("Unknown feature: {}", err)));
+            }
+        }
+    }
+    Ok(())
+}
 
 #[cfg(test)]
 mod tests {