TheR1sing3un commented on code in PR #340:
URL: https://github.com/apache/paimon-rust/pull/340#discussion_r3341808125


##########
crates/paimon/src/spec/schema.rs:
##########
@@ -293,6 +294,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.is_empty())?;

Review Comment:
   Addressed in 7e7a2ae + a59ec09. `AggregationConfig::validate_create_mode` 
now takes `&[DataField]` and rejects three classes of invalid metadata at 
CREATE TABLE:
   
   1. Unknown field name (matches Java `SchemaValidation.validateFieldsPrefix`) 
— error surfaces the typo, the offending option key, and the available columns 
to help spot it.
   2. Unknown function name (matches Java `validateMergeFunctionFactory`) — 
error surfaces the bad name and the supported aggregator list.
   3. Incompatible function/type pair — one step stricter than Java, which 
defers this to `FieldAggregatorFactory#create` at runtime.
   
   The type-compat table lives in `spec/aggregation.rs` because 
`crates/paimon/src/spec/` may not depend on 
`crates/paimon/src/table/aggregator/`. A new test 
`validation_table_matches_constructors` walks every (name, sample-type) pair 
against both the validator and `table::aggregator::new_aggregator`, so the two 
parallel sources of truth cannot drift.
   
   Tests added:
   - 4 unit cases in `aggregation.rs` (unknown field / unknown function / 
incompatible type / unknown default function) + the consistency guard.
   - 3 schema-level cases in `spec/schema.rs` at the `Schema::builder()` layer.
   - 3 e2e cases in `pk_tables.rs` at the DataFusion SQL boundary.
   
   Also note: `fields.default-aggregate-function` only has its name validated; 
per-column type-compat for the default isn't checked because it's broadly 
applied across columns, matching Java's posture.



-- 
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]

Reply via email to