Skip to content

Commit f31e545

Browse files
DrakeLinDrakeLin
authored
test: migrated protocol validation tests to table_configuration (#1517)
## What changes are proposed in this pull request? <!-- Please clarify what changes you are proposing and why the changes are needed. The purpose of this section is to outline the changes, why they are needed, and how this PR fixes the issue. If the reason for the change is already explained clearly in an issue, then it does not need to be restated here. 1. If you propose a new API or feature, clarify the use case for a new API or feature. 2. If you fix a bug, you can clarify why it is a bug. --> Following #1411, move tests over. The previous tests didn't validate protocol, so we had some tests where protocol was 2/7 but writer features didn't exist. Since we construct and validate protocol in the test helper, changed the test to include a writer feature. <!-- Uncomment this section if there are any changes affecting public APIs: ### This PR affects the following public APIs If there are breaking changes, please ensure the `breaking-changes` label gets added by CI, and describe why the changes are needed. Note that _new_ public APIs are not considered breaking. --> ## How was this change tested? <!-- Please make sure to add test cases that check the changes thoroughly including negative and positive cases if possible. If it was tested in a way different from regular unit tests, please clarify how you tested, ideally via a reproducible test documented in the PR description. -->
1 parent ba5e1e3 commit f31e545

File tree

2 files changed

+115
-153
lines changed

2 files changed

+115
-153
lines changed

‎kernel/src/actions/mod.rs‎

Lines changed: 0 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,135 +1477,6 @@ mod tests {
14771477
}
14781478
}
14791479

1480-
use crate::table_configuration::TableConfiguration;
1481-
use crate::utils::test_utils::assert_result_error_with_message;
1482-
1483-
// TODO: Migrate these protocol validation tests to TableConfiguration.
1484-
// The ensure_read_supported and ensure_write_supported methods have been moved from Protocol
1485-
// to TableConfiguration. These tests should be refactored to use TableConfiguration instead.
1486-
1487-
// Helper to create a TableConfiguration for testing protocol validation
1488-
fn create_table_config_for_protocol(protocol: Protocol) -> DeltaResult<TableConfiguration> {
1489-
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
1490-
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new())?;
1491-
let table_root = url::Url::parse("file:///tmp/test").unwrap();
1492-
TableConfiguration::try_new(metadata, protocol, table_root, 0)
1493-
}
1494-
1495-
#[test]
1496-
fn test_v2_checkpoint_supported() {
1497-
let protocol = Protocol::try_new(
1498-
3,
1499-
7,
1500-
Some([TableFeature::V2Checkpoint]),
1501-
Some([TableFeature::V2Checkpoint]),
1502-
)
1503-
.unwrap();
1504-
assert!(create_table_config_for_protocol(protocol).is_ok());
1505-
}
1506-
1507-
#[test]
1508-
fn test_ensure_read_supported() {
1509-
let protocol = Protocol {
1510-
min_reader_version: 3,
1511-
min_writer_version: 7,
1512-
reader_features: Some(vec![]),
1513-
writer_features: Some(vec![]),
1514-
};
1515-
assert!(create_table_config_for_protocol(protocol).is_ok());
1516-
1517-
let protocol = Protocol::try_new(
1518-
3,
1519-
7,
1520-
Some([TableFeature::V2Checkpoint]),
1521-
Some([TableFeature::V2Checkpoint]),
1522-
)
1523-
.unwrap();
1524-
assert!(create_table_config_for_protocol(protocol).is_ok());
1525-
1526-
let protocol = Protocol {
1527-
min_reader_version: 1,
1528-
min_writer_version: 7,
1529-
reader_features: None,
1530-
writer_features: None,
1531-
};
1532-
assert!(create_table_config_for_protocol(protocol).is_ok());
1533-
1534-
let protocol = Protocol {
1535-
min_reader_version: 2,
1536-
min_writer_version: 7,
1537-
reader_features: None,
1538-
writer_features: None,
1539-
};
1540-
assert!(create_table_config_for_protocol(protocol).is_ok());
1541-
}
1542-
1543-
#[test]
1544-
fn test_ensure_write_supported() {
1545-
let protocol = Protocol::try_new(
1546-
3,
1547-
7,
1548-
Some(vec![TableFeature::DeletionVectors]),
1549-
Some(vec![
1550-
TableFeature::AppendOnly,
1551-
TableFeature::DeletionVectors,
1552-
TableFeature::DomainMetadata,
1553-
TableFeature::Invariants,
1554-
TableFeature::RowTracking,
1555-
]),
1556-
)
1557-
.unwrap();
1558-
let config = create_table_config_for_protocol(protocol).unwrap();
1559-
assert!(config.ensure_write_supported().is_ok());
1560-
1561-
// Verify that unsupported reader features are rejected (TypeWidening is a ReaderWriter feature not supported for writes)
1562-
let protocol = Protocol::try_new(
1563-
3,
1564-
7,
1565-
Some([TableFeature::TypeWidening]),
1566-
Some([TableFeature::TypeWidening]),
1567-
)
1568-
.unwrap();
1569-
let config = create_table_config_for_protocol(protocol).unwrap();
1570-
assert_result_error_with_message(
1571-
config.ensure_write_supported(),
1572-
r#"Feature 'typeWidening' not supported for writes"#,
1573-
);
1574-
1575-
// Unknown writer features are allowed during Protocol creation for forward compatibility,
1576-
// but will fail when trying to create TableConfiguration (reads not supported)
1577-
let protocol = Protocol::try_new(
1578-
3,
1579-
7,
1580-
Some([TableFeature::Unknown("unsupported feature".to_string())]),
1581-
Some([TableFeature::Unknown("unsupported feature".to_string())]),
1582-
)
1583-
.unwrap();
1584-
assert_result_error_with_message(
1585-
create_table_config_for_protocol(protocol),
1586-
r#"Unsupported: Unknown feature 'unsupported feature'"#,
1587-
);
1588-
}
1589-
1590-
#[test]
1591-
fn test_illegal_writer_feature_combination() {
1592-
let protocol = Protocol::try_new(
1593-
3,
1594-
7,
1595-
Some::<Vec<String>>(vec![]),
1596-
Some(vec![
1597-
// No domain metadata even though that is required
1598-
TableFeature::RowTracking,
1599-
]),
1600-
)
1601-
.unwrap();
1602-
let config = create_table_config_for_protocol(protocol).unwrap();
1603-
assert_result_error_with_message(
1604-
config.ensure_write_supported(),
1605-
"rowTracking requires domainMetadata to be supported",
1606-
);
1607-
}
1608-
16091480
#[test]
16101481
fn test_ensure_supported_features() {
16111482
let supported_features = [TableFeature::ColumnMapping, TableFeature::DeletionVectors];
@@ -1636,30 +1507,6 @@ mod tests {
16361507
assert_eq!(parse_features::<TableFeature>(features), expected);
16371508
}
16381509

1639-
#[cfg(feature = "catalog-managed")]
1640-
#[test]
1641-
fn test_catalog_managed_writes() {
1642-
let protocol = Protocol::try_new(
1643-
3,
1644-
7,
1645-
Some([TableFeature::CatalogManaged]),
1646-
Some([TableFeature::CatalogManaged]),
1647-
)
1648-
.unwrap();
1649-
let config = create_table_config_for_protocol(protocol).unwrap();
1650-
assert!(config.ensure_write_supported().is_ok());
1651-
1652-
let protocol = Protocol::try_new(
1653-
3,
1654-
7,
1655-
Some([TableFeature::CatalogOwnedPreview]),
1656-
Some([TableFeature::CatalogOwnedPreview]),
1657-
)
1658-
.unwrap();
1659-
let config = create_table_config_for_protocol(protocol).unwrap();
1660-
assert!(config.ensure_write_supported().is_ok());
1661-
}
1662-
16631510
#[test]
16641511
fn test_into_engine_data() {
16651512
let engine = ExprEngine::new();

‎kernel/src/table_configuration.rs‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,4 +1424,119 @@ mod test {
14241424
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
14251425
assert!(!config.is_feature_info_enabled(&feature, &custom_feature_info));
14261426
}
1427+
1428+
#[test]
1429+
fn test_v2_checkpoint_supported() {
1430+
let config = create_mock_table_config(&[], &[TableFeature::V2Checkpoint]);
1431+
assert!(config.is_v2_checkpoint_write_supported());
1432+
}
1433+
1434+
#[test]
1435+
fn test_ensure_read_supported() {
1436+
let config = create_mock_table_config(&[], &[]);
1437+
assert!(config.ensure_read_supported().is_ok());
1438+
1439+
let config = create_mock_table_config(&[], &[TableFeature::V2Checkpoint]);
1440+
assert!(config.ensure_read_supported().is_ok());
1441+
1442+
let config = create_mock_table_config_with_version(&[], None, 1, 2);
1443+
assert!(config.ensure_read_supported().is_ok());
1444+
1445+
let config = create_mock_table_config_with_version(
1446+
&[],
1447+
Some(&[TableFeature::InCommitTimestamp]),
1448+
2,
1449+
7,
1450+
);
1451+
assert!(config.ensure_read_supported().is_ok());
1452+
}
1453+
1454+
#[test]
1455+
fn test_ensure_write_supported() {
1456+
let config = create_mock_table_config(
1457+
&[],
1458+
&[
1459+
TableFeature::AppendOnly,
1460+
TableFeature::DeletionVectors,
1461+
TableFeature::DomainMetadata,
1462+
TableFeature::Invariants,
1463+
TableFeature::RowTracking,
1464+
],
1465+
);
1466+
assert!(config.ensure_write_supported().is_ok());
1467+
1468+
// Type Widening is not supported for writes
1469+
let config = create_mock_table_config(&[], &[TableFeature::TypeWidening]);
1470+
assert_result_error_with_message(
1471+
config.ensure_write_supported(),
1472+
r#"Feature 'typeWidening' not supported for writes"#,
1473+
);
1474+
1475+
// Unknown feature is not supported for reads
1476+
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
1477+
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new()).unwrap();
1478+
let protocol = Protocol::try_new(
1479+
3,
1480+
7,
1481+
Some([TableFeature::Unknown("unsupported feature".to_string())]),
1482+
Some([TableFeature::Unknown("unsupported feature".to_string())]),
1483+
)
1484+
.unwrap();
1485+
let table_root = Url::try_from("file:///").unwrap();
1486+
assert_result_error_with_message(
1487+
TableConfiguration::try_new(metadata, protocol, table_root, 0),
1488+
r#"Unsupported: Unknown feature 'unsupported feature'"#,
1489+
);
1490+
}
1491+
1492+
#[test]
1493+
fn test_illegal_writer_feature_combination() {
1494+
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
1495+
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new()).unwrap();
1496+
let protocol = Protocol::try_new(
1497+
3,
1498+
7,
1499+
Some::<Vec<String>>(vec![]),
1500+
Some(vec![TableFeature::RowTracking]),
1501+
)
1502+
.unwrap();
1503+
let table_root = Url::try_from("file:///").unwrap();
1504+
let config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap();
1505+
assert_result_error_with_message(
1506+
config.ensure_write_supported(),
1507+
"rowTracking requires domainMetadata to be supported",
1508+
);
1509+
}
1510+
1511+
#[test]
1512+
fn test_row_tracking_with_domain_metadata_requirement() {
1513+
let schema = StructType::new_unchecked([StructField::nullable("value", DataType::INTEGER)]);
1514+
let metadata = Metadata::try_new(None, None, schema, vec![], 0, HashMap::new()).unwrap();
1515+
let protocol = Protocol::try_new(
1516+
3,
1517+
7,
1518+
Some::<Vec<String>>(vec![]),
1519+
Some(vec![
1520+
TableFeature::RowTracking,
1521+
TableFeature::DomainMetadata,
1522+
]),
1523+
)
1524+
.unwrap();
1525+
let table_root = Url::try_from("file:///").unwrap();
1526+
let config = TableConfiguration::try_new(metadata, protocol, table_root, 0).unwrap();
1527+
assert!(
1528+
config.ensure_write_supported().is_ok(),
1529+
"RowTracking with DomainMetadata should be supported for writes"
1530+
);
1531+
}
1532+
1533+
#[cfg(feature = "catalog-managed")]
1534+
#[test]
1535+
fn test_catalog_managed_writes() {
1536+
let config = create_mock_table_config(&[], &[TableFeature::CatalogManaged]);
1537+
assert!(config.ensure_write_supported().is_ok());
1538+
1539+
let config = create_mock_table_config(&[], &[TableFeature::CatalogOwnedPreview]);
1540+
assert!(config.ensure_write_supported().is_ok());
1541+
}
14271542
}

0 commit comments

Comments
 (0)