diff --git a/ffi/src/table_changes.rs b/ffi/src/table_changes.rs index a7556d3a2a..9b84e3c00f 100644 --- a/ffi/src/table_changes.rs +++ b/ffi/src/table_changes.rs @@ -372,7 +372,7 @@ mod tests { }} {"protocol": { "minReaderVersion": 1, - "minWriterVersion": 2 + "minWriterVersion": 4 }} {"metaData": { "id": "5fba94ed-9794-4965-ba6e-6ee3c0d22af9", diff --git a/kernel/src/actions/mod.rs b/kernel/src/actions/mod.rs index 0dc4c26cba..914f539b3b 100644 --- a/kernel/src/actions/mod.rs +++ b/kernel/src/actions/mod.rs @@ -2,8 +2,7 @@ //! specification](https://github.com/delta-io/delta/blob/master/PROTOCOL.md) use std::collections::HashMap; -use std::fmt::{Debug, Display}; -use std::hash::Hash; +use std::fmt::Debug; use std::str::FromStr; use std::sync::{Arc, LazyLock}; @@ -24,7 +23,6 @@ use url::Url; use visitors::{MetadataVisitor, ProtocolVisitor}; use delta_kernel_derive::{internal_api, IntoEngineData, ToSchema}; -use itertools::Itertools; use serde::{Deserialize, Serialize}; const KERNEL_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -568,33 +566,6 @@ impl Protocol { self.has_table_feature(&TableFeature::CatalogManaged) || self.has_table_feature(&TableFeature::CatalogOwnedPreview) } - - pub(crate) fn is_cdf_supported(&self) -> bool { - // TODO: we should probably expand this to ~all supported reader features instead of gating - // on a subset here. missing ones are: - // - variantType - // - typeWidening - // - catalogManaged - static CDF_SUPPORTED_READER_FEATURES: LazyLock> = LazyLock::new(|| { - vec![ - TableFeature::DeletionVectors, - TableFeature::ColumnMapping, - TableFeature::TimestampWithoutTimezone, - TableFeature::V2Checkpoint, - TableFeature::VacuumProtocolCheck, - ] - }); - match self.reader_features() { - // if min_reader_version = 3 and all reader features are subset of supported => OK - Some(reader_features) if self.min_reader_version() == 3 => { - ensure_supported_features(reader_features, &CDF_SUPPORTED_READER_FEATURES).is_ok() - } - // if min_reader_version = 1 or 2 and there are no reader features => OK - None => (1..=2).contains(&self.min_reader_version()), - // any other protocol is not supported - _ => false, - } - } } // TODO: implement Scalar::From> so we can derive IntoEngineData using a macro (issue#1083) @@ -636,43 +607,6 @@ impl IntoEngineData for Protocol { } } -// given `table_features`, check if they are subset of `supported_features` -pub(crate) fn ensure_supported_features( - table_features: &[T], - supported_features: &[T], -) -> DeltaResult<()> -where - T: Display + FromStr + Hash + Eq, - ::Err: Display, -{ - // first check if all features are supported, else we proceed to craft an error message - if table_features - .iter() - .all(|feature| supported_features.contains(feature)) - { - return Ok(()); - } - - // we get the type name (TableFeature) for better error messages - let features_type = std::any::type_name::() - .rsplit("::") - .next() - .unwrap_or("table feature"); - - // NB: we didn't do this above to avoid allocation in the common case - let mut unsupported = table_features - .iter() - .filter(|feature| !supported_features.contains(*feature)); - - Err(Error::Unsupported(format!( - "Found unsupported {}s: \"{}\". Supported {}s: \"{}\"", - features_type, - unsupported.join("\", \""), - features_type, - supported_features.iter().join("\", \""), - ))) -} - #[derive(Debug, Clone, PartialEq, Eq, ToSchema)] #[internal_api] #[cfg_attr(test, derive(Serialize, Default), serde(rename_all = "camelCase"))] @@ -1553,21 +1487,6 @@ mod tests { assert!(protocol.validate_table_features().is_err()); } - #[test] - fn test_ensure_supported_features() { - let supported_features = [TableFeature::ColumnMapping, TableFeature::DeletionVectors]; - let table_features = vec![TableFeature::ColumnMapping]; - ensure_supported_features(&table_features, &supported_features).unwrap(); - // test unknown features - let table_features = vec![TableFeature::ColumnMapping, TableFeature::unknown("idk")]; - let error = ensure_supported_features(&table_features, &supported_features).unwrap_err(); - match error { - Error::Unsupported(e) if e == - "Found unsupported TableFeatures: \"idk\". Supported TableFeatures: \"columnMapping\", \"deletionVectors\"" - => {}, - _ => panic!("Expected unsupported error, got: {error}"), - } - } #[test] fn test_parse_table_feature_never_fails() { // parse a non-str diff --git a/kernel/src/checkpoint/mod.rs b/kernel/src/checkpoint/mod.rs index 790749dd07..0cf647034e 100644 --- a/kernel/src/checkpoint/mod.rs +++ b/kernel/src/checkpoint/mod.rs @@ -101,6 +101,7 @@ use crate::log_replay::LogReplayProcessor; use crate::path::ParsedLogPath; use crate::schema::{DataType, SchemaRef, StructField, StructType, ToSchema as _}; use crate::snapshot::SnapshotRef; +use crate::table_features::TableFeature; use crate::table_properties::TableProperties; use crate::{DeltaResult, Engine, EngineData, Error, EvaluationHandlerExtension, FileMeta}; @@ -232,7 +233,7 @@ impl CheckpointWriter { let is_v2_checkpoints_supported = self .snapshot .table_configuration() - .is_v2_checkpoint_write_supported(); + .is_feature_supported(&TableFeature::V2Checkpoint); let actions = self.snapshot.log_segment().read_actions( engine, diff --git a/kernel/src/table_changes/log_replay.rs b/kernel/src/table_changes/log_replay.rs index 3af81b7ca2..195b668b01 100644 --- a/kernel/src/table_changes/log_replay.rs +++ b/kernel/src/table_changes/log_replay.rs @@ -18,9 +18,10 @@ use crate::scan::state::DvInfo; use crate::schema::{ ColumnNamesAndTypes, DataType, SchemaRef, StructField, StructType, ToSchema as _, }; -use crate::table_changes::check_cdf_table_properties; use crate::table_changes::scan_file::{cdf_scan_row_expression, cdf_scan_row_schema}; use crate::table_configuration::TableConfiguration; +use crate::table_features::Operation; +use crate::table_features::TableFeature; use crate::utils::require; use crate::{DeltaResult, Engine, EngineData, Error, PredicateRef, RowVisitor}; @@ -186,15 +187,10 @@ impl LogReplayScanner { visitor.visit_rows_of(actions.as_ref())?; let metadata_opt = Metadata::try_new_from_data(actions.as_ref())?; + let has_metadata_update = metadata_opt.is_some(); let protocol_opt = Protocol::try_new_from_data(actions.as_ref())?; + let has_protocol_update = protocol_opt.is_some(); - // Validate protocol and metadata if present - if let Some(ref protocol) = protocol_opt { - require!( - protocol.is_cdf_supported(), - Error::change_data_feed_unsupported(commit_file.version) - ); - } if let Some(ref metadata) = metadata_opt { let schema = metadata.parse_schema()?; // Currently, schema compatibility is defined as having equal schema types. In the @@ -204,13 +200,10 @@ impl LogReplayScanner { table_schema.as_ref() == &schema, Error::change_data_feed_incompatible_schema(table_schema, &schema) ); - let table_properties = metadata.parse_table_properties(); - check_cdf_table_properties(&table_properties) - .map_err(|_| Error::change_data_feed_unsupported(commit_file.version))?; } // Update table configuration with any new Protocol or Metadata from this commit - if metadata_opt.is_some() || protocol_opt.is_some() { + if has_metadata_update || has_protocol_update { *table_configuration = TableConfiguration::try_new_from( table_configuration, metadata_opt, @@ -218,6 +211,21 @@ impl LogReplayScanner { commit_file.version, )?; } + + // If metadata is updated, check if Change Data Feed is supported + if has_metadata_update { + require!( + table_configuration.is_feature_enabled(&TableFeature::ChangeDataFeed), + Error::change_data_feed_unsupported(commit_file.version) + ); + } + + // If protocol is updated, check if Change Data Feed is supported + if has_protocol_update { + table_configuration + .ensure_operation_supported(Operation::Cdf) + .map_err(|_| Error::change_data_feed_unsupported(commit_file.version))?; + } } // We resolve the remove deletion vector map after visiting the entire commit. if has_cdc_action { diff --git a/kernel/src/table_changes/log_replay/tests.rs b/kernel/src/table_changes/log_replay/tests.rs index b772430986..8cb15f88f5 100644 --- a/kernel/src/table_changes/log_replay/tests.rs +++ b/kernel/src/table_changes/log_replay/tests.rs @@ -41,7 +41,8 @@ fn get_default_table_config(table_root: &url::Url) -> TableConfiguration { ]), ) .unwrap(); - let protocol = Protocol::try_new(1, 1, None::>, None::>).unwrap(); + // CDF requires min_writer_version = 4 + let protocol = Protocol::try_new(1, 4, None::>, None::>).unwrap(); TableConfiguration::try_new(metadata, protocol, table_root.clone(), 0).unwrap() } @@ -167,7 +168,7 @@ async fn metadata_protocol() { 3, 7, Some([TableFeature::DeletionVectors]), - Some([TableFeature::DeletionVectors]), + Some([TableFeature::DeletionVectors, TableFeature::ChangeDataFeed]), ) .unwrap(), ), @@ -230,8 +231,14 @@ async fn unsupported_reader_feature() { Protocol::try_new( 3, 7, - Some([TableFeature::DeletionVectors, TableFeature::TypeWidening]), - Some([TableFeature::DeletionVectors, TableFeature::TypeWidening]), + Some([ + TableFeature::DeletionVectors, + TableFeature::unknown("unsupportedReaderFeature"), + ]), + Some([ + TableFeature::DeletionVectors, + TableFeature::unknown("unsupportedReaderFeature"), + ]), ) .unwrap(), )]) @@ -323,25 +330,24 @@ async fn unsupported_protocol_feature_midstream() { // First commit: Basic protocol with CDF enabled mock_table .commit([ - protocol_action(1, 1, None, None), + protocol_action(2, 6, None, None), metadata_with_cdf(get_schema()), ]) .await; - // Second commit: Protocol update with unsupported feature (TypeWidening) + // Second commit: Protocol update with unsupported feature mock_table .commit([protocol_action( 3, 7, - Some(vec![TableFeature::TypeWidening]), - Some(vec![TableFeature::TypeWidening]), + Some(vec![TableFeature::unknown("unsupportedFeature")]), + Some(vec![TableFeature::unknown("unsupportedFeature")]), )]) .await; assert_midstream_failure(engine, &mock_table); } -// Note: This should be removed once type widening support is added for CDF #[tokio::test] async fn incompatible_schemas_fail() { async fn assert_incompatible_schema(commit_schema: StructType, cdf_schema: StructType) { diff --git a/kernel/src/table_changes/mod.rs b/kernel/src/table_changes/mod.rs index 359ae7a991..6c51e51ec5 100644 --- a/kernel/src/table_changes/mod.rs +++ b/kernel/src/table_changes/mod.rs @@ -42,8 +42,7 @@ use crate::schema::{DataType, Schema, StructField, StructType}; use crate::snapshot::{Snapshot, SnapshotRef}; use crate::table_configuration::TableConfiguration; use crate::table_features::Operation; -use crate::table_properties::TableProperties; -use crate::utils::require; +use crate::table_features::TableFeature; use crate::{DeltaResult, Engine, Error, Version}; mod log_replay; @@ -168,12 +167,12 @@ impl TableChanges { // [`check_cdf_table_properties`] to fail early. This also ensures that column mapping is // disabled. // - // We also check the [`Protocol`] using [`ensure_cdf_read_supported`] to verify that - // we support CDF with those features enabled. - // // Note: We must still check each metadata and protocol action in the CDF range. let check_table_config = |snapshot: &Snapshot| { - if snapshot.table_configuration().is_cdf_read_supported() { + if snapshot + .table_configuration() + .is_feature_enabled(&TableFeature::ChangeDataFeed) + { Ok(()) } else { Err(Error::change_data_feed_unsupported(snapshot.version())) @@ -240,17 +239,6 @@ impl TableChanges { } } -/// Ensures that change data feed is enabled in `table_properties`. See the documentation -/// of [`TableChanges`] for more details. -// TODO: move to TableProperties and normalize with the check in TableConfiguration -fn check_cdf_table_properties(table_properties: &TableProperties) -> DeltaResult<()> { - require!( - table_properties.enable_change_data_feed.unwrap_or(false), - Error::unsupported("Change data feed is not enabled") - ); - Ok(()) -} - #[cfg(test)] mod tests { use super::*; diff --git a/kernel/src/table_configuration.rs b/kernel/src/table_configuration.rs index c81a42edd2..aab505225a 100644 --- a/kernel/src/table_configuration.rs +++ b/kernel/src/table_configuration.rs @@ -41,8 +41,8 @@ pub(crate) enum InCommitTimestampEnablement { /// Holds all the configuration for a table at a specific version. This includes the supported /// reader and writer features, table properties, schema, version, and table root. This can be used /// to check whether a table supports a feature or has it enabled. For example, deletion vector -/// support can be checked with [`TableConfiguration::is_deletion_vector_supported`] and deletion -/// vector write enablement can be checked with [`TableConfiguration::is_deletion_vector_enabled`]. +/// support can be checked with [`TableConfiguration::is_feature_supported`] and deletion +/// vector write enablement can be checked with [`TableConfiguration::is_feature_enabled`]. /// /// [`TableConfiguration`] performs checks upon construction with `TableConfiguration::try_new` /// to validate that Metadata and Protocol are correctly formatted and mutually compatible. @@ -143,6 +143,7 @@ impl TableConfiguration { } /// The [`Protocol`] of this table at this version. + #[allow(unused)] #[internal_api] pub(crate) fn protocol(&self) -> &Protocol { &self.protocol @@ -372,110 +373,6 @@ impl TableConfiguration { Ok(()) } - /// Returns `true` if kernel supports reading Change Data Feed on this table. - /// See the documentation of [`TableChanges`] for more details. - /// - /// [`TableChanges`]: crate::table_changes::TableChanges - #[internal_api] - pub(crate) fn is_cdf_read_supported(&self) -> bool { - let protocol_supported = self.protocol.is_cdf_supported(); - let cdf_enabled = self - .table_properties - .enable_change_data_feed - .unwrap_or(false); - protocol_supported && cdf_enabled - } - - /// Returns `true` if deletion vectors is supported on this table. To support deletion vectors, - /// a table must support reader version 3, writer version 7, and the deletionVectors feature in - /// both the protocol's readerFeatures and writerFeatures. - /// - /// See: - #[internal_api] - #[allow(unused)] // needed to compile w/o default features - pub(crate) fn is_deletion_vector_supported(&self) -> bool { - self.protocol() - .has_table_feature(&TableFeature::DeletionVectors) - && self.protocol.min_reader_version() == 3 - && self.protocol.min_writer_version() == 7 - } - - /// Returns `true` if writing deletion vectors is enabled for this table. This is the case - /// when the deletion vectors is supported on this table and the `delta.enableDeletionVectors` - /// table property is set to `true`. - /// - /// See: - #[internal_api] - #[allow(unused)] // needed to compile w/o default features - pub(crate) fn is_deletion_vector_enabled(&self) -> bool { - self.is_deletion_vector_supported() - && self - .table_properties - .enable_deletion_vectors - .unwrap_or(false) - } - - /// Returns `true` if the table supports the appendOnly table feature. To support this feature: - /// - The table must have a writer version between 2 and 7 (inclusive) - /// - If the table is on writer version 7, it must have the [`TableFeature::AppendOnly`] - /// writer feature. - pub(crate) fn is_append_only_supported(&self) -> bool { - let protocol = &self.protocol; - match protocol.min_writer_version() { - 7 if protocol.has_table_feature(&TableFeature::AppendOnly) => true, - version => (2..=6).contains(&version), - } - } - - #[allow(unused)] - pub(crate) fn is_append_only_enabled(&self) -> bool { - self.is_append_only_supported() && self.table_properties.append_only.unwrap_or(false) - } - - /// Returns `true` if the table supports the column invariant table feature. - #[allow(unused)] - pub(crate) fn is_invariants_supported(&self) -> bool { - let protocol = &self.protocol; - match protocol.min_writer_version() { - 7 if protocol.has_table_feature(&TableFeature::Invariants) => true, - version => (2..=6).contains(&version), - } - } - - /// Returns `true` if V2 checkpoint is supported on this table. To support V2 checkpoint, - /// a table must support reader version 3, writer version 7, and the v2Checkpoint feature in - /// both the protocol's readerFeatures and writerFeatures. - /// - /// See: - pub(crate) fn is_v2_checkpoint_write_supported(&self) -> bool { - self.protocol() - .has_table_feature(&TableFeature::V2Checkpoint) - } - - /// Returns `true` if the table supports writing in-commit timestamps. - /// - /// To support this feature the table must: - /// - Have a min_writer_version of 7 - /// - Have the [`TableFeature::InCommitTimestamp`] writer feature. - #[allow(unused)] - pub(crate) fn is_in_commit_timestamps_supported(&self) -> bool { - self.protocol().min_writer_version() == 7 - && self - .protocol() - .has_table_feature(&TableFeature::InCommitTimestamp) - } - - /// Returns `true` if in-commit timestamps is supported and it is enabled. In-commit timestamps - /// is enabled when the `delta.enableInCommitTimestamps` configuration is set to `true`. - #[allow(unused)] - pub(crate) fn is_in_commit_timestamps_enabled(&self) -> bool { - self.is_in_commit_timestamps_supported() - && self - .table_properties() - .enable_in_commit_timestamps - .unwrap_or(false) - } - /// Returns information about in-commit timestamp enablement state. /// /// Returns an error if only one of the enablement properties is present, as this indicates @@ -484,7 +381,7 @@ impl TableConfiguration { pub(crate) fn in_commit_timestamp_enablement( &self, ) -> DeltaResult { - if !self.is_in_commit_timestamps_enabled() { + if !self.is_feature_enabled(&TableFeature::InCommitTimestamp) { return Ok(InCommitTimestampEnablement::NotEnabled); } @@ -511,42 +408,6 @@ impl TableConfiguration { } } - /// Returns `true` if the table supports writing domain metadata. - /// - /// To support this feature the table must: - /// - Have a min_writer_version of 7. - /// - Have the [`TableFeature::DomainMetadata`] writer feature. - #[allow(unused)] - pub(crate) fn is_domain_metadata_supported(&self) -> bool { - self.protocol().min_writer_version() == 7 - && self - .protocol() - .has_table_feature(&TableFeature::DomainMetadata) - } - - /// Returns `true` if the table supports writing row tracking metadata. - /// - /// To support this feature the table must: - /// - Have a min_writer_version of 7. - /// - Have the [`TableFeature::RowTracking`] writer feature. - pub(crate) fn is_row_tracking_supported(&self) -> bool { - self.protocol().min_writer_version() == 7 - && self - .protocol() - .has_table_feature(&TableFeature::RowTracking) - } - - /// Returns `true` if row tracking is enabled for this table. - /// - /// In order to enable row tracking the table must: - /// - Support row tracking (see [`Self::is_row_tracking_supported`]). - /// - Have the `delta.enableRowTracking` table property set to `true`. - #[allow(unused)] - pub(crate) fn is_row_tracking_enabled(&self) -> bool { - self.is_row_tracking_supported() - && self.table_properties().enable_row_tracking.unwrap_or(false) - } - /// Returns `true` if row tracking is suspended for this table. /// /// Row tracking is suspended when the `delta.rowTrackingSuspended` table property is set to `true`. @@ -568,7 +429,7 @@ impl TableConfiguration { /// Note: We ignore [`is_row_tracking_enabled`] at this point because Kernel does not /// preserve row IDs and row commit versions yet. pub(crate) fn should_write_row_tracking(&self) -> bool { - self.is_row_tracking_supported() && !self.is_row_tracking_suspended() + self.is_feature_supported(&TableFeature::RowTracking) && !self.is_row_tracking_suspended() } /// Returns true if the protocol uses legacy reader version (< 3) @@ -643,7 +504,7 @@ impl TableConfiguration { /// Generic method to check if a feature is supported in the protocol. /// This does NOT check if the feature is enabled via table properties. - #[allow(dead_code)] + #[internal_api] pub(crate) fn is_feature_supported(&self, feature: &TableFeature) -> bool { let Some(info) = feature.info() else { return false; @@ -656,7 +517,7 @@ impl TableConfiguration { /// A feature is enabled if: /// 1. It is supported in the protocol /// 2. The enablement check passes - #[allow(dead_code)] + #[internal_api] pub(crate) fn is_feature_enabled(&self, feature: &TableFeature) -> bool { let Some(info) = feature.info() else { return false; @@ -781,8 +642,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_deletion_vector_supported()); - assert!(!table_config.is_deletion_vector_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::DeletionVectors)); + assert!(!table_config.is_feature_enabled(&TableFeature::DeletionVectors)); } #[test] @@ -812,8 +673,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_deletion_vector_supported()); - assert!(table_config.is_deletion_vector_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::DeletionVectors)); + assert!(table_config.is_feature_enabled(&TableFeature::DeletionVectors)); } #[test] @@ -915,8 +776,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_in_commit_timestamps_supported()); - assert!(table_config.is_in_commit_timestamps_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::InCommitTimestamp)); + assert!(table_config.is_feature_enabled(&TableFeature::InCommitTimestamp)); // When ICT is enabled from table creation (version 0), it's perfectly normal // for enablement properties to be missing let info = table_config.in_commit_timestamp_enablement().unwrap(); @@ -959,8 +820,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_in_commit_timestamps_supported()); - assert!(table_config.is_in_commit_timestamps_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::InCommitTimestamp)); + assert!(table_config.is_feature_enabled(&TableFeature::InCommitTimestamp)); let info = table_config.in_commit_timestamp_enablement().unwrap(); assert_eq!( info, @@ -1000,8 +861,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_in_commit_timestamps_supported()); - assert!(table_config.is_in_commit_timestamps_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::InCommitTimestamp)); + assert!(table_config.is_feature_enabled(&TableFeature::InCommitTimestamp)); assert!(matches!( table_config.in_commit_timestamp_enablement(), Err(Error::Generic(msg)) if msg.contains("In-commit timestamp enabled, but enablement timestamp is missing") @@ -1020,8 +881,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(table_config.is_in_commit_timestamps_supported()); - assert!(!table_config.is_in_commit_timestamps_enabled()); + assert!(table_config.is_feature_supported(&TableFeature::InCommitTimestamp)); + assert!(!table_config.is_feature_enabled(&TableFeature::InCommitTimestamp)); let info = table_config.in_commit_timestamp_enablement().unwrap(); assert_eq!(info, InCommitTimestampEnablement::NotEnabled); } @@ -1057,8 +918,8 @@ mod test { .unwrap(); let table_root = Url::try_from("file:///").unwrap(); let table_config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap(); - assert!(!table_config.is_deletion_vector_supported()); - assert!(!table_config.is_deletion_vector_enabled()); + assert!(!table_config.is_feature_supported(&TableFeature::DeletionVectors)); + assert!(!table_config.is_feature_enabled(&TableFeature::DeletionVectors)); } #[test] @@ -1403,12 +1264,6 @@ mod test { assert!(!config.is_feature_info_enabled(&feature, &custom_feature_info)); } - #[test] - fn test_v2_checkpoint_supported() { - let config = create_mock_table_config(&[], &[TableFeature::V2Checkpoint]); - assert!(config.is_v2_checkpoint_write_supported()); - } - #[test] fn test_ensure_operation_supported_reads() { let config = create_mock_table_config(&[], &[]); diff --git a/kernel/src/transaction/mod.rs b/kernel/src/transaction/mod.rs index 350a196207..f2d5586fc3 100644 --- a/kernel/src/transaction/mod.rs +++ b/kernel/src/transaction/mod.rs @@ -26,7 +26,7 @@ use crate::scan::log_replay::{ use crate::scan::scan_row_schema; use crate::schema::{ArrayType, MapType, SchemaRef, StructField, StructType, StructTypeBuilder}; use crate::snapshot::SnapshotRef; -use crate::table_features::Operation; +use crate::table_features::{Operation, TableFeature}; use crate::utils::{current_time_ms, require}; use crate::{ DataType, DeltaResult, Engine, EngineData, Expression, ExpressionRef, IntoEngineData, @@ -462,7 +462,7 @@ impl Transaction { && !self .read_snapshot .table_configuration() - .is_domain_metadata_supported() + .is_feature_supported(&TableFeature::DomainMetadata) { return Err(Error::unsupported("Domain metadata operations require writer version 7 and the 'domainMetadata' writer feature")); } diff --git a/kernel/tests/data/cdf-column-mapping-name-mode-3-7.tar.zst b/kernel/tests/data/cdf-column-mapping-name-mode-3-7.tar.zst index bb7a606d37..ce7bd9968a 100644 Binary files a/kernel/tests/data/cdf-column-mapping-name-mode-3-7.tar.zst and b/kernel/tests/data/cdf-column-mapping-name-mode-3-7.tar.zst differ diff --git a/kernel/tests/data/cdf-table-with-dv.tar.zst b/kernel/tests/data/cdf-table-with-dv.tar.zst index d3aa6574de..480e784882 100644 Binary files a/kernel/tests/data/cdf-table-with-dv.tar.zst and b/kernel/tests/data/cdf-table-with-dv.tar.zst differ diff --git a/kernel/tests/data/table-with-cdf/_delta_log/00000000000000000000.json b/kernel/tests/data/table-with-cdf/_delta_log/00000000000000000000.json index 09bf8dbb27..0728498906 100644 --- a/kernel/tests/data/table-with-cdf/_delta_log/00000000000000000000.json +++ b/kernel/tests/data/table-with-cdf/_delta_log/00000000000000000000.json @@ -1,4 +1,4 @@ {"commitInfo":{"timestamp":1704392842074,"operation":"Manual Update","operationParameters":{},"isolationLevel":"Serializable","isBlindAppend":true,"operationMetrics":{},"engineInfo":"Apache-Spark/3.5.0 Delta-Lake/3.1.0-SNAPSHOT","txnId":"95ec924a-6859-4433-8008-6d6b4a0e3ba5"}} -{"protocol":{"minReaderVersion":3,"minWriterVersion":7, "readerFeatures":[], "writerFeatures":[]}} +{"protocol":{"minReaderVersion":1,"minWriterVersion":4}} {"metaData":{"id":"testId","format":{"provider":"parquet","options":{}},"schemaString":"{\"type\":\"struct\",\"fields\":[{\"name\":\"part\",\"type\":\"integer\",\"nullable\":true,\"metadata\":{}},{\"name\":\"id\",\"type\":\"integer\",\"nullable\":true,\"metadata\":{}}]}","partitionColumns":[],"configuration":{"delta.enableChangeDataFeed": "true"}}} {"add":{"path":"fake/path/1","partitionValues":{},"size":1,"modificationTime":1,"dataChange":true}}