JunRuiLee commented on PR #7832:
URL: https://github.com/apache/paimon/pull/7832#issuecomment-4526171203
Thanks @JingsongLi for the detailed review.
I addressed the actionable safety/documentation/test items in this update:
1. Added documentation for snapshot-ordering compaction: compacted records
intentionally store snapshot id in the sequence field, so within-snapshot
secondary ordering is not preserved after compaction. This is
safe under the current invariant because compaction has already resolved
same-key conflicts.
2. Added a comment in `PartialUpdateMergeFunction` for field-level
sequence groups: the output row has one row-level snapshot id, which may
overstate recency for fields retained from older inputs. This matches
the existing `latestSequenceNumber` behavior.
3. Added a fail-fast guard when snapshot-ordering reads a file without
`fileSource` metadata. `sequence.snapshot-ordering` is immutable and should
only be enabled for newly-created/empty tables, so legacy files
should not reach this path.
4. Added defensive checks that `sequence.snapshot-ordering` is not used
together with a user-defined sequence comparator near the sort/lookup
comparator creation paths.
5. Added Javadoc for `validateTableSchema(schema, dynamicOptionKeys)` to
document the dynamic `write-only` override contract for dedicated compaction
jobs.
6. The existing integration tests use the default `LOSER_TREE`; I added a
`MIN_HEAP` variant so both sort engines are covered.
I did not include the broader comparator-strategy refactor in this PR
because it would touch several merge paths and make the correctness-focused
review larger. I added fail-fast checks and documentation around the current
paths instead, and I think the strategy refactor is better handled as a
follow-up cleanup.
--
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]
