zhuqi-lucas commented on code in PR #24354:
URL: https://github.com/apache/datafusion/pull/24354#discussion_r3781256735
##########
datafusion/datasource-parquet/src/push_decoder.rs:
##########
@@ -342,6 +342,15 @@ impl PushDecoderStreamState {
.as_ref()
.expect("decoder present")
.is_at_row_group_boundary();
+ // Before pruning/rebuilding, align `rg_plan` with the row group
the
+ // decoder will actually emit next. arrow-rs silently finishes row
+ // groups whose post-predicate selection is empty without handing
back
+ // a reader, so without this sync `rg_plan` trails the decoder by
one
+ // and a later rebuild can re-read an already-delivered row group
+ // (#24352).
+ if at_boundary && let Err(e) =
self.sync_rg_plan_to_decoder_frontier() {
Review Comment:
Good catch — done in 44f7fc7. Gated the sync on
`self.row_group_pruner.is_some()`: only the runtime pruner rebuilds the decoder
from `rg_plan`, so only it needs the sync, and this keeps ordinary scans (no
pruner, never rebuild) from paying the `peek_next_row_group()` cost at every
boundary.
##########
datafusion/sqllogictest/test_files/dynamic_row_group_pruning.slt:
##########
@@ -110,3 +110,70 @@ RESET datafusion.execution.parquet.pushdown_filters;
statement ok
RESET datafusion.explain.analyze_level;
+
+# Regression test for #24352: TopK dynamic filter + `pushdown_filters` must not
Review Comment:
Agreed — done in 44f7fc7. The slt now enables both dynamic-filter switches
explicitly instead of relying on defaults. For asserting the prune/rebuild path
is actually taken, I added that to the companion Rust integration test
(`topk_pushdown_does_not_reread_delivered_row_group` in
`dynamic_row_group_pruning.rs`), which asserts
`row_groups_pruned_dynamic_filter >= 1`. I went with the metric assertion there
rather than a full `EXPLAIN` plan in the slt — the metric is a more robust
guard against future optimizer/default changes and avoids a brittle plan
snapshot. Happy to add an slt `EXPLAIN` too if you prefer belt-and-suspenders.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
