JingsongLi commented on code in PR #340:
URL: https://github.com/apache/paimon-rust/pull/340#discussion_r3410618489
##########
crates/paimon/src/spec/schema.rs:
##########
@@ -305,6 +306,7 @@ impl Schema {
let fields = Self::normalize_fields(&fields, &partition_keys,
&primary_keys)?;
Self::validate_blob_fields(&fields, &partition_keys, &options)?;
PartialUpdateConfig::new(&options).validate_create_mode(!primary_keys.is_empty())?;
+ AggregationConfig::new(&options).validate_create_mode(&primary_keys,
&fields)?;
Review Comment:
Could we run this same aggregation validation from
`TableSchema::apply_changes` after applying `SetOption` / `RemoveOption`?
`CREATE TABLE` now rejects typoed `fields.<col>.aggregate-function` keys,
unknown function names, and function/type mismatches, but `ALTER TABLE ... SET
TBLPROPERTIES` goes through `apply_changes` and currently only revalidates the
first-row changelog constraint. That can still persist invalid aggregation
metadata after table creation; for example `fields.amout.aggregate-function =
sum` would be accepted by ALTER and then silently fall back to
`last_non_null_value` at read time. Reusing
`AggregationConfig::validate_create_mode(&new_schema.primary_keys,
&new_schema.fields)` before saving the altered schema would close that gap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
