leaves12138 commented on code in PR #831:
URL: https://github.com/apache/paimon-rust/pull/831#discussion_r4006286015
##########
crates/paimon/src/table/table_scan.rs:
##########
@@ -1827,11 +1949,26 @@ impl<'a> PaimonTableScan<'a> {
let open_file_cost = core_options.source_split_open_file_cost();
let partition_keys = self.table.schema().partition_keys();
+ // Assign row positions using the full candidate file ranges, before
+ // group stats, projection or DVs change visible rows. Intersect
explicit
+ // and index-selected ranges only after assigning the positional range.
+ let effective_row_ranges = if let Some(selection) =
self.row_position_selection {
+ Some(selection.select(&entries, effective_row_ranges.as_deref())?)
Review Comment:
[P1] Assign row positions before row-range pruning
`entries` is already pruned by explicit/global-index row ranges here:
`plan_snapshot_from_lists` passes `manifest_row_ranges` into
`plan_manifest_entries_with_trace`, which applies `retain_manifest_row_ranges`
and `retain_manifest_entry_row_ranges`. If that removes a preceding file,
`selection.select` renumbers the remaining rows, so intersecting the ranges
afterward cannot preserve the requested positions.
For example, persist a DE snapshot with two three-row files covering row IDs
`[0,2]` and `[3,5]`, then run:
```python
rb.with_row_ranges([(4, 4)])
plan = rb.new_scan().with_row_position_slice(3, 6).plan()
```
The positional slice should select IDs `[3,5]`, and its intersection with
`[4,4]` should retain row 4. Instead, early pruning removes the first file, the
remaining three rows are assigned positions `[0,3)`, and the returned plan has
no splits. I reproduced this through the public Rust `new_scan().plan()` path
against this head; the expected `[RowRange { from: 4, to: 4 }]` was actually
`[]`.
Please preserve the complete candidate position space when positional
selection is enabled, and only then intersect explicit/index-selected ranges.
Disabling the early row-range pruning for positional scans made the reproducer
and the existing row-position tests pass locally. Add multi-file coverage where
the explicit range removes an entire preceding file, including combined
incremental scans and shard selection; the current intersection tests do not
exercise that case.
--
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]
