JingsongLi commented on PR #9966: URL: https://github.com/apache/paimon/pull/9966#issuecomment-5997093375
After reviewing `0991ccc`, I consider the implementation risk substantial and recommend a smaller correctness fix before merging. The original bug is important: projecting a partially populated BLOB/vector column can silently omit logical rows. However, the conditional anchor-removal logic now has to reproduce contracts across full snapshots, DELTA scans, per-column coverage, row-range pushdown, and independently rolled dedicated files. The two reproduced regressions show that these boundaries are not yet handled safely: [streaming DELTA planning fails](https://github.com/apache/paimon/pull/9966#discussion_r4185389909), and [fully covered rolled vector groups repeatedly read the normal file](https://github.com/apache/paimon/pull/9966#discussion_r4185389936) (20 times the normal-file I/O in the measured case). The benefit of conditionally dropping the anchor has not been demonstrated. Retaining its metadata does not necessarily read its data: the existing column planner skips opening bunches that contribute no requested fields. That makes the additional coverage machinery difficult to justify for this correctness fix. I suggest narrowing the change to: 1. When a group already contains a normal file and pruning would leave only dedicated files, conservatively retain the oldest normal anchor. Preserve dedicated-only DELTA groups without requiring an anchor. This avoids the new per-column/requested-range coverage calculation in the scan. 2. Keep the vector NULL-filling fix, but enter the range-aware path only for genuinely incomplete projected column-bunch coverage. Fully covered rolled vector bunches should retain the existing sequential path. 3. Move conditional anchor removal into a separate optimization PR, supported by measured savings and explicit batch/incremental compatibility tests. Please verify the smaller fix with end-to-end full-snapshot and DELTA reads, single/multiple BLOB columns, partial and fully covered rolled vector groups, row-range pushdown, and deletion vectors. The original correctness fix is valuable; the current scope should be reduced to make its contracts easier to verify. -- 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]
