Skip to content

Commit 5a4fbda

Browse files
committed
fix(spec): validate sequence.field against table schema
Creating a table with `sequence.field` referencing a missing column was silently accepted, then the write path resolved sequence fields with a lenient lookup and fell back to the auto-increment sequence, ignoring the user's ordering intent. Repeated fields, `merge-engine=first-row` and cross-partition update tables were likewise unchecked. Validate `sequence.field` at create and alter time, mirroring Java `SchemaValidation#validateSequenceField`: every listed field must exist, must not repeat, first-row merge engine rejects user-defined sequence fields, and cross-partition update tables (primary key constraint not including all partition fields) reject `sequence.field` because partition migration retracts old rows with generated DELETEs whose ordering a user-provided sequence value could break.
1 parent 009d610 commit 5a4fbda

1 file changed

Lines changed: 215 additions & 0 deletions

File tree

crates/paimon/src/spec/schema.rs

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,12 @@ impl TableSchema {
502502
&new_schema.partition_keys,
503503
&new_schema.primary_keys,
504504
)?;
505+
Schema::validate_sequence_field(
506+
&new_schema.options,
507+
&new_schema.fields,
508+
&new_schema.partition_keys,
509+
&new_schema.primary_keys,
510+
)?;
505511
Schema::validate_read_batch_size(&new_schema.options)?;
506512
Ok(new_schema)
507513
}
@@ -979,6 +985,7 @@ impl Schema {
979985
Self::validate_first_row_changelog_producer(&options)?;
980986
Self::validate_rowkind_field(&options, &primary_keys, &fields)?;
981987
Self::validate_bucket_keys(&options, &fields, &partition_keys, &primary_keys)?;
988+
Self::validate_sequence_field(&options, &fields, &partition_keys, &primary_keys)?;
982989
Self::validate_read_batch_size(&options)?;
983990

984991
Ok(Self {
@@ -1461,6 +1468,69 @@ impl Schema {
14611468
.map_err(Self::options_error_to_config_invalid)
14621469
}
14631470

1471+
/// Validate the `sequence.field` option against the schema, mirroring
1472+
/// Java `SchemaValidation#validateSequenceField`:
1473+
/// * every listed field must exist in the table schema — otherwise the
1474+
/// write path silently falls back to the auto-increment sequence
1475+
/// (`TableWrite` resolves sequence fields with a lenient lookup) and
1476+
/// merge results would ignore the user's ordering intent;
1477+
/// * a field must not be listed more than once;
1478+
/// * `merge-engine=first-row` does not support user-defined sequence
1479+
/// fields;
1480+
/// * cross-partition update tables (primary key constraint not including
1481+
/// all partition fields) do not support user-defined sequence fields,
1482+
/// because partition migration retracts old rows with generated DELETEs
1483+
/// whose ordering a user-provided sequence value could break.
1484+
fn validate_sequence_field(
1485+
options: &HashMap<String, String>,
1486+
fields: &[DataField],
1487+
partition_keys: &[String],
1488+
primary_keys: &[String],
1489+
) -> crate::Result<()> {
1490+
let core = CoreOptions::new(options);
1491+
let sequence_fields = core.sequence_fields();
1492+
if sequence_fields.is_empty() {
1493+
return Ok(());
1494+
}
1495+
1496+
let mut seen: HashSet<&str> = HashSet::new();
1497+
for name in &sequence_fields {
1498+
if fields.iter().all(|f| f.name() != *name) {
1499+
return Err(crate::Error::ConfigInvalid {
1500+
message: format!("Sequence field '{name}' can not be found in table schema."),
1501+
});
1502+
}
1503+
if !seen.insert(name) {
1504+
return Err(crate::Error::ConfigInvalid {
1505+
message: format!("Sequence field '{name}' is defined repeatedly."),
1506+
});
1507+
}
1508+
}
1509+
1510+
let merge_engine = core
1511+
.merge_engine()
1512+
.map_err(Self::options_error_to_config_invalid)?;
1513+
if merge_engine == MergeEngine::FirstRow {
1514+
return Err(crate::Error::ConfigInvalid {
1515+
message: "Do not support use sequence field on FIRST_ROW merge engine.".to_string(),
1516+
});
1517+
}
1518+
1519+
let cross_partition_update = !primary_keys.is_empty()
1520+
&& !partition_keys.is_empty()
1521+
&& partition_keys.iter().any(|pt| !primary_keys.contains(pt));
1522+
if cross_partition_update {
1523+
return Err(crate::Error::ConfigInvalid {
1524+
message: format!(
1525+
"You can not use sequence.field in cross partition update case \
1526+
(Primary key constraint '{primary_keys:?}' not include all partition fields '{partition_keys:?}')."
1527+
),
1528+
});
1529+
}
1530+
1531+
Ok(())
1532+
}
1533+
14641534
/// Returns top-level Blob field names for create-time Blob contract checks.
14651535
fn top_level_blob_field_names(fields: &[DataField]) -> Vec<&str> {
14661536
fields
@@ -3273,6 +3343,151 @@ mod tests {
32733343
);
32743344
}
32753345

3346+
#[test]
3347+
fn test_create_schema_rejects_unknown_sequence_field() {
3348+
let err = Schema::builder()
3349+
.column("id", DataType::Int(IntType::new()))
3350+
.column("ts", DataType::Int(IntType::new()))
3351+
.primary_key(["id"])
3352+
.option("sequence.field", "no_such_col")
3353+
.build()
3354+
.unwrap_err();
3355+
3356+
assert!(
3357+
matches!(err, crate::Error::ConfigInvalid { ref message }
3358+
if message.contains("no_such_col") && message.contains("can not be found")),
3359+
"sequence.field referencing a missing column should be rejected, got {err:?}"
3360+
);
3361+
}
3362+
3363+
#[test]
3364+
fn test_create_schema_rejects_repeated_sequence_field() {
3365+
let err = Schema::builder()
3366+
.column("id", DataType::Int(IntType::new()))
3367+
.column("ts", DataType::Int(IntType::new()))
3368+
.primary_key(["id"])
3369+
.option("sequence.field", "ts,ts")
3370+
.build()
3371+
.unwrap_err();
3372+
3373+
assert!(
3374+
matches!(err, crate::Error::ConfigInvalid { ref message }
3375+
if message.contains("ts") && message.contains("repeatedly")),
3376+
"repeated sequence.field should be rejected, got {err:?}"
3377+
);
3378+
}
3379+
3380+
#[test]
3381+
fn test_create_schema_rejects_sequence_field_with_first_row() {
3382+
let err = Schema::builder()
3383+
.column("id", DataType::Int(IntType::new()))
3384+
.column("ts", DataType::Int(IntType::new()))
3385+
.primary_key(["id"])
3386+
.option("merge-engine", "first-row")
3387+
.option("sequence.field", "ts")
3388+
.build()
3389+
.unwrap_err();
3390+
3391+
assert!(
3392+
matches!(err, crate::Error::ConfigInvalid { ref message }
3393+
if message.contains("FIRST_ROW")),
3394+
"sequence.field with merge-engine=first-row should be rejected, got {err:?}"
3395+
);
3396+
}
3397+
3398+
#[test]
3399+
fn test_alter_set_unknown_sequence_field_rejected() {
3400+
let table_schema = TableSchema::new(
3401+
0,
3402+
&Schema::builder()
3403+
.column("id", DataType::Int(IntType::new()))
3404+
.column("ts", DataType::Int(IntType::new()))
3405+
.primary_key(["id"])
3406+
.build()
3407+
.unwrap(),
3408+
);
3409+
3410+
let err = table_schema
3411+
.apply_changes(vec![crate::spec::SchemaChange::set_option(
3412+
"sequence.field".to_string(),
3413+
"no_such_col".to_string(),
3414+
)])
3415+
.unwrap_err();
3416+
3417+
assert!(
3418+
matches!(err, crate::Error::ConfigInvalid { ref message }
3419+
if message.contains("no_such_col") && message.contains("can not be found")),
3420+
"alter setting sequence.field to a missing column should be rejected, got {err:?}"
3421+
);
3422+
}
3423+
3424+
#[test]
3425+
fn test_create_schema_rejects_sequence_field_with_cross_partition_update() {
3426+
// PK (id) does not include the partition field (pt): cross-partition
3427+
// update case, where user-defined sequence fields are not supported.
3428+
let err = Schema::builder()
3429+
.column("pt", DataType::Int(IntType::new()))
3430+
.column("id", DataType::Int(IntType::new()))
3431+
.column("ts", DataType::Int(IntType::new()))
3432+
.partition_keys(["pt"])
3433+
.primary_key(["id"])
3434+
.option("sequence.field", "ts")
3435+
.build()
3436+
.unwrap_err();
3437+
3438+
assert!(
3439+
matches!(err, crate::Error::ConfigInvalid { ref message }
3440+
if message.contains("cross partition update")),
3441+
"sequence.field with cross-partition update should be rejected, got {err:?}"
3442+
);
3443+
}
3444+
3445+
#[test]
3446+
fn test_create_schema_accepts_sequence_field_when_pk_covers_partitions() {
3447+
// PK includes the partition field: not a cross-partition update case.
3448+
let schema = Schema::builder()
3449+
.column("pt", DataType::Int(IntType::new()))
3450+
.column("id", DataType::Int(IntType::new()))
3451+
.column("ts", DataType::Int(IntType::new()))
3452+
.partition_keys(["pt"])
3453+
.primary_key(["pt", "id"])
3454+
.option("sequence.field", "ts")
3455+
.build();
3456+
3457+
assert!(
3458+
schema.is_ok(),
3459+
"sequence.field with PK covering partition fields should be accepted, got {schema:?}"
3460+
);
3461+
}
3462+
3463+
#[test]
3464+
fn test_alter_set_sequence_field_with_cross_partition_update_rejected() {
3465+
let table_schema = TableSchema::new(
3466+
0,
3467+
&Schema::builder()
3468+
.column("pt", DataType::Int(IntType::new()))
3469+
.column("id", DataType::Int(IntType::new()))
3470+
.column("ts", DataType::Int(IntType::new()))
3471+
.partition_keys(["pt"])
3472+
.primary_key(["id"])
3473+
.build()
3474+
.unwrap(),
3475+
);
3476+
3477+
let err = table_schema
3478+
.apply_changes(vec![crate::spec::SchemaChange::set_option(
3479+
"sequence.field".to_string(),
3480+
"ts".to_string(),
3481+
)])
3482+
.unwrap_err();
3483+
3484+
assert!(
3485+
matches!(err, crate::Error::ConfigInvalid { ref message }
3486+
if message.contains("cross partition update")),
3487+
"alter setting sequence.field on cross-partition update table should be rejected, got {err:?}"
3488+
);
3489+
}
3490+
32763491
#[test]
32773492
fn test_drop_column_referenced_by_sequence_field_rejected() {
32783493
let table_schema = TableSchema::new(

0 commit comments

Comments
 (0)