JingsongLi commented on PR #10162: URL: https://github.com/apache/paimon/pull/10162#issuecomment-5832037269
This PR has real end-to-end value for supported aggregation tables: a same-key `SUM` batch now returns 60 instead of silently keeping the final value 30. I reviewed the write-buffer fold and `AggregateMergeFunction` construction and ran 76 aggregation, merge-buffer, and sequence tests locally; the head's Python and Native CI is green. `git diff --check` also passed. **P1 — floating `sequence.field` still makes the result depend on write grouping.** On this exact head I created two identical aggregation tables with a `DOUBLE` sequence field and `last_value` labels. I wrote `(seq=1.0, label='finite')` then `(seq=NaN, label='nan')`. Writing both rows in one batch returned `finite`; committing them separately returned `nan`. The new `_sort_by_primary_key` uses Arrow floating sort, while the read comparator follows a different NaN order. Because the buffer collapses the two rows before commit, the lost winner cannot be recovered by a later read. Please integrate the Java-compatible floating sequence ordering from #10166 into this head and add this one-batch versus separate-commit regression. **Production dependency — unsupported aggregation options still write using deduplicate fallback.** This head's `_build_pk_merge_function` explicitly permits that fallback and its test expects a warning after writing. A later read error cannot recover the discarded input. #10165 adds early rejection; please integrate or make it a required predecessor before this PR merges, then verify the combined writer construction and supported aggregation read/write path. Until these two dependencies are present on the merged head, I would not ship this change despite the green CI. -- 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]
