leaves12138 commented on code in PR #890:
URL: https://github.com/apache/paimon-rust/pull/890#discussion_r4058629978
##########
crates/paimon/src/table/data_file_reader.rs:
##########
@@ -310,13 +313,19 @@ impl DataFileReader {
FileIndexResult::Remain
};
+ let range_base = if data_evolution {
+ file_meta.first_row_id.unwrap_or(0)
+ } else {
+ split_file_offset
Review Comment:
[P1] Preserve ordinary row-ID selections on row-tracking append tables
The existing planner still places absolute row IDs into splits for
`ReadBuilder.with_row_ranges(...)`: `split_row_ranges_for_files` intersects the
requested IDs with each file's `first_row_id` range. This branch now interprets
every non-DE split as split-local physical offsets, including ordinary
row-tracking scans that never call `with_chunk_shuffle`. The planner and reader
therefore use different coordinate systems.
Reproduced with this PR's actual Python binding: create an append table with
`row-tracking.enabled=true` (leave data evolution disabled), set both
`source.split.target-size` and `source.split.open-file-cost` to `1b`, and make
two commits containing IDs `[10, 11, 12]` and `[20, 21, 22]`. Then read the
splits from:
```python
builder = table.new_read_builder().with_row_ranges([(3, 4)])
splits = builder.new_scan().plan().splits()
batches = list(builder.new_read().read(splits))
```
The control runtime returns IDs `[20, 21]`; this PR returns no batches.
Selecting `[(1, 4)]` similarly returns only `[11, 12]` instead of `[11, 12, 20,
21]`. The second file has `first_row_id=3`, but its split-local offset is zero,
so its selected rows are discarded. I also verified the base-branch data-file
reader matches the control reader.
Please make the existing row-ID planning path and the new raw chunk path
agree on coordinates (for example, translate absolute selections into
split-local ranges when constructing raw splits), and add a regression test
with multiple row-tracking files whose first row IDs are nonzero. This does not
require retaining an unpublished wire format, but must avoid silently changing
the results of the existing user-facing API.
--
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]
