zhuqi-lucas commented on code in PR #24354:
URL: https://github.com/apache/datafusion/pull/24354#discussion_r3781668903
##########
datafusion/datasource-parquet/src/push_decoder.rs:
##########
@@ -414,6 +428,49 @@ impl PushDecoderStreamState {
}
}
+ /// Keep `rg_plan.front()` aligned with the row group the decoder will emit
+ /// next. `try_next_reader` silently finishes row groups whose
post-predicate
+ /// selection is empty (no reader handed back), which would otherwise leave
+ /// `rg_plan` trailing the decoder by one — a later prune/rebuild would
then
+ /// re-include an already-delivered row group (#24352).
+ fn sync_rg_plan_to_decoder_frontier(&mut self) -> Result<(),
DataFusionError> {
+ match self
+ .decoder
+ .as_ref()
+ .expect("decoder present")
+ .peek_next_row_group()
+ .map_err(DataFusionError::from)?
+ {
+ Some(actual) => self.advance_rg_plan_to(actual)?,
+ // Decoder has nothing left to emit — drain our plan so the stream
+ // finishes cleanly.
+ None => self.rg_plan.clear(),
+ }
+ Ok(())
+ }
+
+ /// Pop `rg_plan` entries until its front is `target`.
+ ///
+ /// `target` is the RG the decoder will emit next and must still be in our
+ /// plan. A missing `target` means the decoder's frontier and `rg_plan`
+ /// have diverged; we surface that as an internal error rather than
+ /// silently draining the plan, which would truncate the scan.
+ fn advance_rg_plan_to(&mut self, target: usize) -> Result<()> {
Review Comment:
Added coverage in bddf975: `advance_rg_plan_to` is now a free function over
`rg_plan`, with two unit tests — pop-up-to-target, and the guard erroring when
the target is absent. Both fail under the `==`→`!=` mutation you flagged. And
filed #24358 for the structural fix (`retain_row_groups`) that removes
`rg_plan` and this whole coupling.
--
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]
