[python] Reject unsupported aggregation options before writing - #10165
TheR1sing3un wants to merge 2 commits into
Conversation
d88e800 to
d846b29
Compare
|
Reviewed the current head as a data-loss prevention change. It has standalone end-to-end value for unsupported aggregation configurations: writer construction now fails before a row can enter the buffer or dynamic-bucket extractor, so a later read error cannot be the first indication that input was discarded. I checked the construction order ( Local verification: 91 relevant aggregation, partial-update, sequence, write-buffer, and native-dispatch tests passed; 14 optional-Rust cases were skipped and 4 subtests passed. The current CI is green for Native CI and all scheduled Python versions. The earlier dynamic-bucket side-effect and false-flag test issues have been addressed at this head. No new blocker found in this guard. Production scope to keep visible: this PR does not make normal aggregation writes safe by itself. |
d846b29 to
6c50dd9
Compare
Purpose
Aggregation tables configured with unsupported options can commit data using deduplicate semantics. A later read error cannot recover the discarded input. Validate these options when constructing the writer, before dynamic bucket assignment can retain hashes for rejected rows.
Reuse the read-side guard for unsupported retract options, sequence groups, aggregator identifiers and invalid sequence configurations. Explicitly false retract flags remain writable. This guard alone does not provide correct writes for supported aggregation configurations: those still use the existing deduplicate fallback. #10162 is required for supported aggregation write semantics; please keep that dependency explicit when merging. Native read validation of false-valued retract flags remains a separate compatibility gap.
Tests
66e2530a7(merged [python] Honor sequence.field in primary-key writes #10166), retaining both sequence and aggregation validation before row-key extraction. PyArrow 16.0.0 / NumPy 1.26.4: 179 aggregation, partial-update, sequence read/write, merge-buffer and dispatch tests passed, plus 4 subtests. Changed-file lint, Python 3.6 syntax and diff checks passed.git diff --checkpassed.Combined validation
Validated #10162 (
8002ab54c), this PR (d846b29aa) and #10166 (2605de145) together in an isolated checkout. The integration retains this PR's early aggregation guard, #10166's early sequence validation and IEEE-aware ordering, and #10162's supported aggregation dispatch. The overlapping late-validation/fallback expectations from #10162 are replaced by the early-rejection tests.test_sum_aggregates_same_key_rows_in_one_batch(10 + 20 + 30 = 60, with another key preserved) andtest_sequence_field_honored_within_one_batch.The combined implementation is verification-only; this PR remains scoped to early rejection of unsupported configurations.