leaves12138 commented on PR #839: URL: https://github.com/apache/paimon-rust/pull/839#issuecomment-5676883276
Non-blocking, pre-existing follow-up found during the cross-reader validation: the Rust aggregation reader can still aggregate a partition primary-key column. Reproduction: create a table with columns `(k BIGINT, p INT, amount BIGINT)`, primary key `(k, p)`, partition key `p`, `bucket=1`, `merge-engine=aggregation`, and `fields.default-aggregate-function=sum`. Commit `(1, 7, 10)` and `(1, 7, 20)` separately, with exactly one row in each data file. The current Rust SQL reader returns `(1, 14, 30)`, while both Java and PyPaimon return `(1, 7, 30)`. The write side in this PR correctly passes full primary keys. However, `crates/paimon/src/table/table_read.rs` still populates `KeyValueReadConfig.primary_keys` with `trimmed_primary_keys()`, and `kv_file_reader.rs::new_merge_function` passes those names to `AggregateMergeFunction::new`. Consequently `p` is treated as a value column and summed when files are merged. The read-side aggregation constructor needs the full key names while physical-key comparison should continue using the trimmed keys. I also reproduced the same result with the pre-fix binding from #838, and the relevant read-side production wiring is unchanged by this PR. This is therefore a separate existing issue, not a regression or a blocker for these DE/flush fixes. Please cover a partitioned aggregation table across at least two commits in the follow-up; a single-file physical-row assertion does not exercise this reader path. -- 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]
