leaves12138 commented on PR #9177: URL: https://github.com/apache/paimon/pull/9177#issuecomment-5254574905
I found a correctness issue in the two-phase planning flow when a dedicated file spans multiple normal row-id ranges. A `RangeBatch` may contain multiple independent candidate ranges. The second scan uses `withRowRanges`, which includes every file that *intersects* any selected range, and then passes all returned entries to one `CompactPlanner` invocation. A spanning blob/vector file can therefore reconnect candidate ranges that were intentionally separated by a non-candidate normal range. `CompactPlanner` builds its outer range groups from all files, including dedicated files, so it may produce a normal compaction task across a row-id gap. A minimal example is: - normal `[0, 4]`: two small files, plus two updated blob files spanning `[0, 12]` (blob candidate) - normal `[5, 9]`: one oversized non-candidate file - normal `[10, 19]`: four small files (normal candidate) The candidate collector correctly returns `[0, 4]` and `[10, 19]`. However, the second scan also returns the `[0, 12]` blobs. They reconnect both ranges inside `CompactPlanner`, which then generates a normal task containing files from `[0, 4]` and `[10, 19]`, while excluding `[5, 9]`. `DataEvolutionNormalCompactTask` concatenates these disjoint ranges and assigns the output the first input row ID. This shifts the latter rows and can create an output range overlapping the untouched `[5, 9]` file. The commit-side existence check does not necessarily reject this because the new range is still contained in the original overall coverage. I reproduced this deterministically and also found it with a 20,000-iteration differential test comparing: 1. `CompactPlanner` on all entries; and 2. candidate collection -> row-range filtering -> `CompactPlanner`. The normal-only differential test passed, while the blob version found this mismatch. I think candidate boundaries must be preserved in phase two. One possible fix is to group normal files by their own contiguous row-id coverage first, and only then attach blob/vector files to their anchor normal range, so dedicated files cannot define normal-range connectivity. It would also be valuable to add `checkContiguousRowRange(compactBefore)` to `DataEvolutionNormalCompactTask` as a defensive safeguard. Two additional concerns: - `data-evolution.compaction.rewrite-row-ids=true` now fails with guidance to use a separate deletion-vector materialization operation, but I could not find an exposed Flink/Spark action or procedure providing that replacement. This changes an existing capability into a hard failure without a migration path. - Each candidate batch rescans the complete manifest group. For legacy manifests without row-id bounds, all manifests form one group, so more than 100,000 candidate files may cause repeated full-manifest scans. The 100,000-file value is also an estimated candidate count rather than a strict bound on full metadata returned by the intersecting-range scan. -- 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]
