fix(spec): reject aggregation on a sequence field for every merge engine - #638
Open
jackylee-ch wants to merge 1 commit into
Open
fix(spec): reject aggregation on a sequence field for every merge engine#638jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
Java `SchemaValidation#validateSequenceField` checks `options.fieldAggFunc(field) == null` for every field listed in `sequence.field`, and that validator runs regardless of the configured merge engine. The Rust equivalent lived inside `AggregationConfig::validate_field_scoped_options`, which is only reached when `merge-engine=aggregation`, so on the default deduplicate engine a schema combining `sequence.field = 'ts'` with `fields.ts.aggregate-function = 'sum'` was silently accepted and persisted. Hoist the check into a free `validate_no_aggregation_on_sequence_field` keyed only on the option map, and call it from `Schema::new` and `TableSchema::apply_changes` next to the other sequence-field validations, so it no longer depends on the merge engine.
jackylee-ch
force-pushed
the
fix/agg-on-sequence-field
branch
from
July 31, 2026 12:23
ce21f0b to
9a1683b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Java
SchemaValidation#validateSequenceFieldchecksfieldAggFunc(field) == nullfor every field in
sequence.field, and that validator runs for every mergeengine. The Rust equivalent lived inside
AggregationConfig::validate_field_scoped_options, which is only reached whenmerge-engine=aggregation— so on the default deduplicate engine a schemacombining
sequence.field = 'ts'withfields.ts.aggregate-function = 'sum'wassilently accepted and persisted.
Fix: hoist the check into a free
validate_no_aggregation_on_sequence_fieldkeyed only on the option map, and call it from
Schema::newandTableSchema::apply_changesalongside the other sequence-field validations, so itno longer depends on the merge engine. The existing aggregation-engine test is
widened to cover all five engines, plus create/alter tests on the default engine.