Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 0 additions & 153 deletions kernel/src/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1448,135 +1448,6 @@ mod tests {
}
}

use crate::table_configuration::TableConfiguration;
use crate::utils::test_utils::assert_result_error_with_message;

// TODO: Migrate these protocol validation tests to TableConfiguration.
// The ensure_read_supported and ensure_write_supported methods have been moved from Protocol
// to TableConfiguration. These tests should be refactored to use TableConfiguration instead.

// Helper to create a TableConfiguration for testing protocol validation
fn create_table_config_for_protocol(protocol: Protocol) -> DeltaResult<TableConfiguration> {
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new())?;
let table_root = url::Url::parse("file:///tmp/test").unwrap();
TableConfiguration::try_new(metadata, protocol, table_root, 0)
}

#[test]
fn test_v2_checkpoint_supported() {
let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::V2Checkpoint]),
Some([TableFeature::V2Checkpoint]),
)
.unwrap();
assert!(create_table_config_for_protocol(protocol).is_ok());
}

#[test]
fn test_ensure_read_supported() {
let protocol = Protocol {
min_reader_version: 3,
min_writer_version: 7,
reader_features: Some(vec![]),
writer_features: Some(vec![]),
};
assert!(create_table_config_for_protocol(protocol).is_ok());

let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::V2Checkpoint]),
Some([TableFeature::V2Checkpoint]),
)
.unwrap();
assert!(create_table_config_for_protocol(protocol).is_ok());

let protocol = Protocol {
min_reader_version: 1,
min_writer_version: 7,
reader_features: None,
writer_features: None,
};
assert!(create_table_config_for_protocol(protocol).is_ok());

let protocol = Protocol {
min_reader_version: 2,
min_writer_version: 7,
reader_features: None,
writer_features: None,
};
assert!(create_table_config_for_protocol(protocol).is_ok());
}

#[test]
fn test_ensure_write_supported() {
let protocol = Protocol::try_new(
3,
7,
Some(vec![TableFeature::DeletionVectors]),
Some(vec![
TableFeature::AppendOnly,
TableFeature::DeletionVectors,
TableFeature::DomainMetadata,
TableFeature::Invariants,
TableFeature::RowTracking,
]),
)
.unwrap();
let config = create_table_config_for_protocol(protocol).unwrap();
assert!(config.ensure_write_supported().is_ok());

// Verify that unsupported reader features are rejected (TypeWidening is a ReaderWriter feature not supported for writes)
let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::TypeWidening]),
Some([TableFeature::TypeWidening]),
)
.unwrap();
let config = create_table_config_for_protocol(protocol).unwrap();
assert_result_error_with_message(
config.ensure_write_supported(),
r#"Feature 'typeWidening' not supported for writes"#,
);

// Unknown writer features are allowed during Protocol creation for forward compatibility,
// but will fail when trying to create TableConfiguration (reads not supported)
let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::Unknown("unsupported feature".to_string())]),
Some([TableFeature::Unknown("unsupported feature".to_string())]),
)
.unwrap();
assert_result_error_with_message(
create_table_config_for_protocol(protocol),
r#"Unsupported: Unknown feature 'unsupported feature'"#,
);
}

#[test]
fn test_illegal_writer_feature_combination() {
let protocol = Protocol::try_new(
3,
7,
Some::<Vec<String>>(vec![]),
Some(vec![
// No domain metadata even though that is required
TableFeature::RowTracking,
]),
)
.unwrap();
let config = create_table_config_for_protocol(protocol).unwrap();
assert_result_error_with_message(
config.ensure_write_supported(),
"rowTracking requires domainMetadata to be supported",
);
}

#[test]
fn test_ensure_supported_features() {
let supported_features = [TableFeature::ColumnMapping, TableFeature::DeletionVectors];
Expand Down Expand Up @@ -1607,30 +1478,6 @@ mod tests {
assert_eq!(parse_features::<TableFeature>(features), expected);
}

#[cfg(feature = "catalog-managed")]
#[test]
fn test_catalog_managed_writes() {
let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::CatalogManaged]),
Some([TableFeature::CatalogManaged]),
)
.unwrap();
let config = create_table_config_for_protocol(protocol).unwrap();
assert!(config.ensure_write_supported().is_ok());

let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::CatalogOwnedPreview]),
Some([TableFeature::CatalogOwnedPreview]),
)
.unwrap();
let config = create_table_config_for_protocol(protocol).unwrap();
assert!(config.ensure_write_supported().is_ok());
}

#[test]
fn test_into_engine_data() {
let engine = ExprEngine::new();
Expand Down
93 changes: 93 additions & 0 deletions kernel/src/table_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1439,4 +1439,97 @@ mod test {
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
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_read_supported() {
let config = create_mock_table_config(&[], &[]);
assert!(config.ensure_read_supported().is_ok());

let config = create_mock_table_config(&[], &[TableFeature::V2Checkpoint]);
assert!(config.ensure_read_supported().is_ok());

let config = create_mock_table_config_with_version(&[], None, 1, 2);
assert!(config.ensure_read_supported().is_ok());

let config = create_mock_table_config_with_version(
&[],
Some(&[TableFeature::InCommitTimestamp]),
2,
7,
);
assert!(config.ensure_read_supported().is_ok());
}

#[test]
fn test_ensure_write_supported() {
let config = create_mock_table_config(
&[],
&[
TableFeature::AppendOnly,
TableFeature::DeletionVectors,
TableFeature::DomainMetadata,
TableFeature::Invariants,
TableFeature::RowTracking,
],
);
assert!(config.ensure_write_supported().is_ok());

// Type Widening is not supported for writes
let config = create_mock_table_config(&[], &[TableFeature::TypeWidening]);
assert_result_error_with_message(
config.ensure_write_supported(),
r#"Feature 'typeWidening' not supported for writes"#,
);

// Unknown feature is not supported for writes
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new()).unwrap();
let protocol = Protocol::try_new(
3,
7,
Some([TableFeature::Unknown("unsupported feature".to_string())]),
Some([TableFeature::Unknown("unsupported feature".to_string())]),
)
.unwrap();
let table_root = Url::try_from("file:///").unwrap();
assert_result_error_with_message(
TableConfiguration::try_new(metadata, protocol, table_root, 0),
r#"Unsupported: Unknown feature 'unsupported feature'"#,
);
}

#[test]
fn test_illegal_writer_feature_combination() {
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new()).unwrap();
let protocol = Protocol::try_new(
3,
7,
Some::<Vec<String>>(vec![]),
Some(vec![TableFeature::RowTracking]),
)
.unwrap();
let table_root = Url::try_from("file:///").unwrap();
let config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap();
assert_result_error_with_message(
config.ensure_write_supported(),
"rowTracking requires domainMetadata to be supported",
);
Comment on lines +1518 to +1523
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a case for RowTracking + Domain metadata to show that adding the requirement fixes it?

}

#[cfg(feature = "catalog-managed")]
#[test]
fn test_catalog_managed_writes() {
let config = create_mock_table_config(&[], &[TableFeature::CatalogManaged]);
assert!(config.ensure_write_supported().is_ok());

let config = create_mock_table_config(&[], &[TableFeature::CatalogOwnedPreview]);
assert!(config.ensure_write_supported().is_ok());
}
}
Loading