Skip to content

Commit 1c29adf

Browse files
authored
feat: add generic is_feature_supported and is_feature_enabled methods to TableConfiguration (#1405)
## What changes are proposed in this pull request? This PR adds generic `is_feature_supported()` and `is_feature_enabled()` methods to `TableConfiguration`, utilizing FeatureInfo added previously ## 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. --> New tests
1 parent 743018b commit 1c29adf

File tree

1 file changed

+317
-16
lines changed

1 file changed

+317
-16
lines changed

kernel/src/table_configuration.rs

Lines changed: 317 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::schema::variant_utils::validate_variant_type_feature_support;
1717
use crate::schema::{InvariantChecker, SchemaRef};
1818
use crate::table_features::{
1919
column_mapping_mode, validate_schema_column_mapping, validate_timestamp_ntz_feature_support,
20-
ColumnMappingMode, TableFeature,
20+
ColumnMappingMode, EnablementCheck, FeatureInfo, FeatureType, TableFeature,
2121
};
2222
use crate::table_properties::TableProperties;
2323
use crate::utils::require;
@@ -435,6 +435,99 @@ impl TableConfiguration {
435435
pub(crate) fn should_write_row_tracking(&self) -> bool {
436436
self.is_row_tracking_supported() && !self.is_row_tracking_suspended()
437437
}
438+
439+
/// Returns true if the protocol uses legacy reader version (< 3)
440+
#[allow(dead_code)]
441+
fn is_legacy_reader_version(&self) -> bool {
442+
self.protocol.min_reader_version() < 3
443+
}
444+
445+
/// Returns true if the protocol uses legacy writer version (< 7)
446+
#[allow(dead_code)]
447+
fn is_legacy_writer_version(&self) -> bool {
448+
self.protocol.min_writer_version() < 7
449+
}
450+
451+
/// Helper to check if a feature is present in a feature list.
452+
fn has_feature(features: Option<&[TableFeature]>, feature: &TableFeature) -> bool {
453+
features
454+
.map(|features| features.contains(feature))
455+
.unwrap_or(false)
456+
}
457+
458+
/// Helper method to check if a feature is supported based on its FeatureInfo.
459+
/// This checks protocol versions and feature lists but does NOT check enablement properties.
460+
#[allow(dead_code)]
461+
fn is_feature_info_supported(&self, feature: &TableFeature, info: &FeatureInfo) -> bool {
462+
match info.feature_type {
463+
FeatureType::Writer => {
464+
if self.is_legacy_writer_version() {
465+
// Legacy writer: protocol writer version meets minimum requirement
466+
self.protocol.min_writer_version() >= info.min_writer_version
467+
} else {
468+
// Table features writer: feature is in writer_features list
469+
Self::has_feature(self.protocol.writer_features(), feature)
470+
}
471+
}
472+
FeatureType::ReaderWriter => {
473+
let reader_supported = if self.is_legacy_reader_version() {
474+
// Legacy reader: protocol reader version meets minimum requirement
475+
self.protocol.min_reader_version() >= info.min_reader_version
476+
} else {
477+
// Table features reader: feature is in reader_features list
478+
Self::has_feature(self.protocol.reader_features(), feature)
479+
};
480+
481+
let writer_supported = if self.is_legacy_writer_version() {
482+
// Legacy writer: protocol writer version meets minimum requirement
483+
self.protocol.min_writer_version() >= info.min_writer_version
484+
} else {
485+
// Table features writer: feature is in writer_features list
486+
Self::has_feature(self.protocol.writer_features(), feature)
487+
};
488+
489+
reader_supported && writer_supported
490+
}
491+
FeatureType::Unknown => false,
492+
}
493+
}
494+
495+
/// Helper method to check if a feature is enabled based on its FeatureInfo.
496+
/// This checks both protocol support and enablement via table properties.
497+
#[allow(dead_code)]
498+
fn is_feature_info_enabled(&self, feature: &TableFeature, info: &FeatureInfo) -> bool {
499+
if !self.is_feature_info_supported(feature, info) {
500+
return false;
501+
}
502+
503+
match info.enablement_check {
504+
EnablementCheck::AlwaysIfSupported => true,
505+
EnablementCheck::EnabledIf(check_fn) => check_fn(&self.table_properties),
506+
}
507+
}
508+
509+
/// Generic method to check if a feature is supported in the protocol.
510+
/// This does NOT check if the feature is enabled via table properties.
511+
#[allow(dead_code)]
512+
pub(crate) fn is_feature_supported(&self, feature: &TableFeature) -> bool {
513+
let Some(info) = feature.info() else {
514+
return false;
515+
};
516+
self.is_feature_info_supported(feature, info)
517+
}
518+
519+
/// Generic method to check if a feature is enabled.
520+
///
521+
/// A feature is enabled if:
522+
/// 1. It is supported in the protocol
523+
/// 2. The enablement check passes
524+
#[allow(dead_code)]
525+
pub(crate) fn is_feature_enabled(&self, feature: &TableFeature) -> bool {
526+
let Some(info) = feature.info() else {
527+
return false;
528+
};
529+
self.is_feature_info_enabled(feature, info)
530+
}
438531
}
439532

440533
#[cfg(test)]
@@ -445,7 +538,9 @@ mod test {
445538

446539
use crate::actions::{Metadata, Protocol};
447540
use crate::schema::{DataType, StructField, StructType};
448-
use crate::table_features::{FeatureType, TableFeature};
541+
use crate::table_features::{
542+
EnablementCheck, FeatureInfo, FeatureType, KernelSupport, TableFeature,
543+
};
449544
use crate::table_properties::TableProperties;
450545
use crate::utils::test_utils::assert_result_error_with_message;
451546
use crate::Error;
@@ -456,7 +551,7 @@ mod test {
456551
props_to_enable: &[&str],
457552
features: &[TableFeature],
458553
) -> TableConfiguration {
459-
create_mock_table_config_with_version(props_to_enable, features.into(), 3, 7)
554+
create_mock_table_config_with_version(props_to_enable, Some(features), 3, 7)
460555
}
461556

462557
fn create_mock_table_config_with_version(
@@ -479,25 +574,51 @@ mod test {
479574
),
480575
)
481576
.unwrap();
482-
let reader_features = features_opt.map(|features| {
483-
features
577+
578+
let (reader_features_opt, writer_features_opt) = if let Some(features) = features_opt {
579+
let reader_features = features
484580
.iter()
485581
.filter(|feature| matches!(feature.feature_type(), FeatureType::ReaderWriter))
486-
});
487-
let writer_features = features_opt.map(|features| {
488-
features.iter().filter(|feature| {
489-
matches!(
490-
feature.feature_type(),
491-
FeatureType::Writer | FeatureType::ReaderWriter
492-
)
493-
})
494-
});
582+
.cloned()
583+
.collect::<Vec<_>>();
584+
let writer_features = features
585+
.iter()
586+
.filter(|feature| {
587+
matches!(
588+
feature.feature_type(),
589+
FeatureType::Writer | FeatureType::ReaderWriter
590+
)
591+
})
592+
.cloned()
593+
.collect::<Vec<_>>();
594+
(
595+
// Only add reader_features if reader >= 3 (non-legacy reader mode)
596+
// Protocol requires Some (even if empty) when reader = 3
597+
if min_reader_version >= 3 {
598+
Some(reader_features)
599+
} else {
600+
None
601+
},
602+
// Only add writer_features if writer >= 7 (non-legacy writer mode)
603+
// Protocol requires Some (even if empty) when writer = 7
604+
if min_writer_version >= 7 {
605+
Some(writer_features)
606+
} else {
607+
None
608+
},
609+
)
610+
} else {
611+
(None, None)
612+
};
613+
614+
let reader_features_iter = reader_features_opt.as_ref().map(|f| f.iter());
615+
let writer_features_iter = writer_features_opt.as_ref().map(|f| f.iter());
495616

496617
let protocol = Protocol::try_new(
497618
min_reader_version,
498619
min_writer_version,
499-
reader_features,
500-
writer_features,
620+
reader_features_iter,
621+
writer_features_iter,
501622
)
502623
.unwrap();
503624
let table_root = Url::try_from("file:///").unwrap();
@@ -966,4 +1087,184 @@ mod test {
9661087
"Should succeed when VARIANT is used with required features"
9671088
);
9681089
}
1090+
1091+
#[test]
1092+
fn test_is_feature_supported_returns_false_without_info() {
1093+
// is_feature_supported should return false for features without FeatureInfo
1094+
let config = create_mock_table_config(&[], &[TableFeature::DeletionVectors]);
1095+
assert!(!config.is_feature_supported(&TableFeature::unknown("futureFeature")));
1096+
}
1097+
1098+
#[test]
1099+
fn test_is_feature_enabled_returns_false_without_info() {
1100+
// is_feature_enabled should return false for features without FeatureInfo
1101+
let config = create_mock_table_config(&[], &[TableFeature::DeletionVectors]);
1102+
assert!(!config.is_feature_enabled(&TableFeature::unknown("futureFeature")));
1103+
}
1104+
1105+
#[test]
1106+
fn test_is_feature_info_supported_writer() {
1107+
// Use ColumnMapping (a ReaderWriter feature) with custom FeatureInfo as Writer type
1108+
let feature = TableFeature::ColumnMapping;
1109+
1110+
// Custom FeatureInfo that treats ColumnMapping as Writer-only with min_writer_version = 2
1111+
let custom_feature_info = FeatureInfo {
1112+
name: "columnMapping",
1113+
min_reader_version: 1,
1114+
min_writer_version: 2,
1115+
feature_type: FeatureType::Writer,
1116+
feature_requirements: &[],
1117+
read_support: KernelSupport::Supported,
1118+
write_support: KernelSupport::Supported,
1119+
enablement_check: EnablementCheck::AlwaysIfSupported,
1120+
};
1121+
1122+
// Test with legacy protocol writer v2 - should be supported
1123+
let config = create_mock_table_config_with_version(&[], None, 1, 2);
1124+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1125+
1126+
// Test with legacy protocol writer v1 - should NOT be supported
1127+
let config = create_mock_table_config_with_version(&[], None, 1, 1);
1128+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1129+
1130+
// Test with asymmetric: reader=2 (legacy), writer=7 (non-legacy)
1131+
// For this to work with a Writer-only FeatureInfo, we need a real Writer-only feature
1132+
// Use AppendOnly instead of ColumnMapping for the 2,7 test cases
1133+
let writer_only_feature = TableFeature::AppendOnly;
1134+
let writer_only_info = FeatureInfo {
1135+
name: "appendOnly",
1136+
min_reader_version: 1,
1137+
min_writer_version: 2,
1138+
feature_type: FeatureType::Writer,
1139+
feature_requirements: &[],
1140+
read_support: KernelSupport::Supported,
1141+
write_support: KernelSupport::Supported,
1142+
enablement_check: EnablementCheck::AlwaysIfSupported,
1143+
};
1144+
1145+
// reader=2 (legacy), writer=7 (non-legacy) - feature in list, should be supported
1146+
let config =
1147+
create_mock_table_config_with_version(&[], Some(&[TableFeature::AppendOnly]), 2, 7);
1148+
assert!(config.is_feature_info_supported(&writer_only_feature, &writer_only_info));
1149+
1150+
// reader=2 (legacy), writer=7 (non-legacy) - feature NOT in list, should NOT be supported
1151+
// Use ChangeDataFeed which is also a Writer-only feature
1152+
let config =
1153+
create_mock_table_config_with_version(&[], Some(&[TableFeature::ChangeDataFeed]), 2, 7);
1154+
assert!(!config.is_feature_info_supported(&writer_only_feature, &writer_only_info));
1155+
1156+
// Test with protocol reader=3, writer=7 (both non-legacy) - feature in list, should be supported
1157+
let config = create_mock_table_config(&[], &[TableFeature::ColumnMapping]);
1158+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1159+
1160+
// Test with protocol reader=3, writer=7 (both non-legacy) - feature NOT in list, should NOT be supported
1161+
let config = create_mock_table_config(&[], &[TableFeature::DeletionVectors]);
1162+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1163+
}
1164+
1165+
#[test]
1166+
fn test_is_feature_info_supported_reader_writer() {
1167+
// Use ColumnMapping (a real ReaderWriter feature) with custom FeatureInfo
1168+
// ColumnMapping is a legacy feature (reader=2, writer=5) which makes it ideal for
1169+
// testing both legacy mode (version checks) and non-legacy mode (feature list checks)
1170+
let feature = TableFeature::ColumnMapping;
1171+
1172+
// Custom FeatureInfo that requires reader=2, writer=5
1173+
let custom_feature_info = FeatureInfo {
1174+
name: "columnMapping",
1175+
min_reader_version: 2,
1176+
min_writer_version: 5,
1177+
feature_type: FeatureType::ReaderWriter,
1178+
feature_requirements: &[],
1179+
read_support: KernelSupport::Supported,
1180+
write_support: KernelSupport::Supported,
1181+
enablement_check: EnablementCheck::AlwaysIfSupported,
1182+
};
1183+
1184+
// Test with sufficient versions (legacy mode) - should be supported
1185+
let config = create_mock_table_config_with_version(&[], None, 2, 5);
1186+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1187+
1188+
// Test with insufficient reader version - should NOT be supported
1189+
let config = create_mock_table_config_with_version(&[], None, 1, 5);
1190+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1191+
1192+
// Test with insufficient writer version - should NOT be supported
1193+
let config = create_mock_table_config_with_version(&[], None, 2, 4);
1194+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1195+
1196+
// Test with asymmetric: reader=2 (legacy), writer=7 (non-legacy)
1197+
// ReaderWriter features CANNOT be enabled in this protocol state (protocol validation)
1198+
// But we still need to test that the code correctly identifies them as NOT supported
1199+
// Create a table with only Writer-only features (e.g., AppendOnly)
1200+
let config =
1201+
create_mock_table_config_with_version(&[], Some(&[TableFeature::AppendOnly]), 2, 7);
1202+
// ColumnMapping (ReaderWriter) should NOT be supported because:
1203+
// - reader=2 (legacy) checks version: 2 >= 2 ✓ (reader_supported = true)
1204+
// - writer=7 (non-legacy) checks list: ColumnMapping not in writer_features ✗ (writer_supported = false)
1205+
// - Result: false (requires BOTH to be true)
1206+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1207+
1208+
// Test with non-legacy mode (3,7) - feature in list, should be supported
1209+
let config = create_mock_table_config(&[], &[TableFeature::ColumnMapping]);
1210+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1211+
1212+
// Test with non-legacy mode (3,7) - feature NOT in list, should NOT be supported
1213+
let config = create_mock_table_config(&[], &[TableFeature::DeletionVectors]);
1214+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1215+
}
1216+
1217+
#[test]
1218+
fn test_is_feature_info_enabled_with_custom_property_check() {
1219+
// Create a custom feature with a property check function
1220+
let custom_feature_info = FeatureInfo {
1221+
name: "customPropertyFeature",
1222+
min_reader_version: 1,
1223+
min_writer_version: 2,
1224+
feature_type: FeatureType::Writer,
1225+
feature_requirements: &[],
1226+
read_support: KernelSupport::Supported,
1227+
write_support: KernelSupport::Supported,
1228+
enablement_check: EnablementCheck::EnabledIf(|props| props.append_only == Some(true)),
1229+
};
1230+
1231+
let feature = TableFeature::unknown("customPropertyFeature");
1232+
1233+
// Test when property check fails - should be supported but not enabled
1234+
let config = create_mock_table_config_with_version(&[], None, 1, 2);
1235+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1236+
assert!(!config.is_feature_info_enabled(&feature, &custom_feature_info));
1237+
1238+
// Test when property check passes - should be both supported and enabled
1239+
let config = create_mock_table_config_with_version(&["delta.appendOnly"], None, 1, 2);
1240+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1241+
assert!(config.is_feature_info_enabled(&feature, &custom_feature_info));
1242+
}
1243+
1244+
#[test]
1245+
fn test_is_feature_info_enabled_always_if_supported() {
1246+
// Create a custom feature that's always enabled if supported
1247+
let custom_feature_info = FeatureInfo {
1248+
name: "alwaysEnabledFeature",
1249+
min_reader_version: 1,
1250+
min_writer_version: 3,
1251+
feature_type: FeatureType::Writer,
1252+
feature_requirements: &[],
1253+
read_support: KernelSupport::Supported,
1254+
write_support: KernelSupport::Supported,
1255+
enablement_check: EnablementCheck::AlwaysIfSupported,
1256+
};
1257+
1258+
let feature = TableFeature::unknown("alwaysEnabledFeature");
1259+
1260+
// Test when supported - should be both supported and enabled
1261+
let config = create_mock_table_config_with_version(&[], None, 1, 3);
1262+
assert!(config.is_feature_info_supported(&feature, &custom_feature_info));
1263+
assert!(config.is_feature_info_enabled(&feature, &custom_feature_info));
1264+
1265+
// Test when not supported - should be neither supported nor enabled
1266+
let config = create_mock_table_config_with_version(&[], None, 1, 2);
1267+
assert!(!config.is_feature_info_supported(&feature, &custom_feature_info));
1268+
assert!(!config.is_feature_info_enabled(&feature, &custom_feature_info));
1269+
}
9691270
}

0 commit comments

Comments
 (0)