zhuqi-lucas opened a new pull request, #24354:
URL: https://github.com/apache/datafusion/pull/24354

   ## Which issue does this PR close?
   
   - Closes #24352.
   
   ## Rationale for this change
   
   With `datafusion.execution.parquet.pushdown_filters = true` and TopK dynamic 
filter pushdown (both on by default), a query of the shape `SELECT b FROM t 
WHERE <predicate on a> ORDER BY b LIMIT k` can silently return **wrong 
results** — one source row emitted several times and the true tail of the top-k 
missing — with no error or warning.
   
   Root cause (thanks to @hhhizzz's very detailed report + fixture in #24352): 
a row group whose post-predicate selection is empty is silently finished by 
arrow-rs **without handing back a reader**. `PushDecoderStreamState` pops its 
`rg_plan` **only** when a reader is returned, so after a silently-finished RG 
the plan trails the decoder by one. When the runtime row-group pruner then 
rebuilds the decoder (`into_builder().with_row_groups(...)`) from the stale 
`rg_plan`, it re-includes an already-delivered row group, whose rows are 
emitted a second time and displace the genuine top-k in the heap.
   
   ## What changes are included in this PR?
   
   - `push_decoder.rs`: before each boundary prune/rebuild, `rg_plan` is synced 
to the row group the decoder will actually emit next via 
`peek_next_row_group()` (`sync_rg_plan_to_decoder_frontier` / 
`advance_rg_plan_to`), dropping entries for silently-finished row groups so a 
rebuild can never re-include a delivered group. A rebuild frontier naming an RG 
not in the plan is now an internal error instead of a silent plan drain.
   
   ## Are these changes tested?
   
   - Adds @hhhizzz's fixture as an slt regression test in 
`dynamic_row_group_pruning.slt` (filter column `search_phrase` differs from the 
sort column `event_time`, one row group has an empty post-predicate selection 
invisible to statistics). It now returns the correct `p0 p4096 p4097 … p4104` 
(was the buggy `p0 p4096 p4096 …`).
   - clippy clean; `datasource-parquet` unit tests and the sqllogictest suite 
pass locally.
   
   ## Are there any user-facing changes?
   
   Fixes silently-wrong query results; no API change.
   
   ## Note
   
   This is the standalone bug fix extracted from #23696 (per review discussion 
in #24352): the same `rg_plan` ↔ decoder-frontier sync, on its own so it merges 
fast and is easy to backport. #23696 will rebase on top so it carries only the 
fully-matched `RowFilter` skip performance optimization.
   
   cc @alamb @adriangb @hhhizzz
   


-- 
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]

Reply via email to