Skip to content

Commit 741ad22

Browse files
committed
Add protocol validation check
1 parent eaac1df commit 741ad22

File tree

2 files changed

+42
-9
lines changed

2 files changed

+42
-9
lines changed

kernel/src/actions/mod.rs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,18 @@ impl Protocol {
469469
match &self.writer_features {
470470
Some(writer_features) if self.min_writer_version == 7 => {
471471
// if we're on version 7, make sure we support all the specified features
472-
ensure_supported_features(writer_features, &SUPPORTED_WRITER_FEATURES)
472+
ensure_supported_features(writer_features, &SUPPORTED_WRITER_FEATURES)?;
473+
474+
// ensure that there is no illegal combination of features
475+
if writer_features.contains(&WriterFeature::RowTracking)
476+
&& !writer_features.contains(&WriterFeature::DomainMetadata)
477+
{
478+
Err(Error::invalid_protocol(
479+
"rowTracking feature requires domainMetadata to also be enabled",
480+
))
481+
} else {
482+
Ok(())
483+
}
473484
}
474485
Some(_) => {
475486
// there are features, but we're not on 7, so the protocol is actually broken
@@ -914,15 +925,17 @@ impl DomainMetadata {
914925
mod tests {
915926
use super::*;
916927
use crate::{
917-
arrow::array::{
918-
Array, BooleanArray, Int32Array, Int64Array, ListArray, ListBuilder, MapBuilder,
919-
MapFieldNames, RecordBatch, StringArray, StringBuilder, StructArray,
928+
arrow::{
929+
array::{
930+
Array, BooleanArray, Int32Array, Int64Array, ListArray, ListBuilder, MapBuilder,
931+
MapFieldNames, RecordBatch, StringArray, StringBuilder, StructArray,
932+
},
933+
datatypes::{DataType as ArrowDataType, Field, Schema},
934+
json::ReaderBuilder,
920935
},
921-
arrow::datatypes::{DataType as ArrowDataType, Field, Schema},
922-
arrow::json::ReaderBuilder,
923-
engine::arrow_data::ArrowEngineData,
924-
engine::arrow_expression::ArrowEvaluationHandler,
936+
engine::{arrow_data::ArrowEngineData, arrow_expression::ArrowEvaluationHandler},
925937
schema::{ArrayType, DataType, MapType, StructField},
938+
utils::test_utils::assert_result_error_with_message,
926939
Engine, EvaluationHandler, JsonHandler, ParquetHandler, StorageHandler,
927940
};
928941
use serde_json::json;
@@ -1315,6 +1328,7 @@ mod tests {
13151328
Some(vec![
13161329
WriterFeature::AppendOnly,
13171330
WriterFeature::DeletionVectors,
1331+
WriterFeature::DomainMetadata,
13181332
WriterFeature::Invariants,
13191333
WriterFeature::RowTracking,
13201334
]),
@@ -1323,6 +1337,25 @@ mod tests {
13231337
assert!(protocol.ensure_write_supported().is_ok());
13241338
}
13251339

1340+
#[test]
1341+
fn test_illegal_writer_feature_combination() {
1342+
let protocol = Protocol::try_new(
1343+
3,
1344+
7,
1345+
Some::<Vec<String>>(vec![]),
1346+
Some(vec![
1347+
// No domain metadata even though that is required
1348+
WriterFeature::RowTracking,
1349+
]),
1350+
)
1351+
.unwrap();
1352+
1353+
assert_result_error_with_message(
1354+
protocol.ensure_write_supported(),
1355+
"rowTracking feature requires domainMetadata to also be enabled",
1356+
);
1357+
}
1358+
13261359
#[test]
13271360
fn test_ensure_supported_features() {
13281361
let supported_features = [ReaderFeature::ColumnMapping, ReaderFeature::DeletionVectors];

kernel/tests/row_tracking.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ async fn create_row_tracking_table(
4141
&[], // no partition columns
4242
true, // use 37 protocol
4343
vec![], // no reader features
44-
vec!["rowTracking"],
44+
vec!["domainMetadata", "rowTracking"],
4545
)
4646
.await
4747
.map_err(|e| Error::generic(format!("Failed to create table: {e}")))?;

0 commit comments

Comments
 (0)