zhuqi-lucas commented on code in PR #24509:
URL: https://github.com/apache/datafusion/pull/24509#discussion_r3822665172
##########
datafusion/datasource-parquet/src/access_plan.rs:
##########
@@ -1049,6 +1100,41 @@ mod test {
/// [`RowGroupMetaData`] that returns 4 row groups with 10, 20, 30, 40 rows
/// respectively
+ #[test]
+ fn test_strip_empty_row_groups_drops_only_empties() {
+ // 4 row groups of [10, 20, 30, 40] rows. RG 1 and RG 3 select nothing
+ // after pruning, so they must be dropped; RG 0 and RG 2 survive with
+ // their re-concatenated selections intact.
+ let selection = RowSelection::from(vec![
+ RowSelector::select(10), // RG 0: keep all 10
+ RowSelector::skip(30), // RG 1 (20) fully skipped + RG 2's
leading 10
+ RowSelector::select(20), // RG 2: keep 20
+ RowSelector::skip(40), // RG 3: skip all 40
+ ]);
+
+ let (indexes, result) = strip_empty_row_groups(
+ vec![0, 1, 2, 3],
+ Some(selection),
+ &ROW_GROUP_METADATA,
+ );
+
+ // RG 1 and RG 3 are dropped; the surviving indexes stay in order.
+ assert_eq!(indexes, vec![0, 2]);
+ let result = result.expect("survivors keep a selection");
+ assert_eq!(result.row_count(), 30); // 10 from RG 0 + 20 from RG 2
+ assert_eq!(result.skipped_row_count(), 10); // RG 2's leading skip
+ }
+
+ #[test]
+ fn test_strip_empty_row_groups_none_selection_unchanged() {
+ // With no row selection no row group can be empty, so the inputs pass
+ // through unchanged.
+ let (indexes, result) =
+ strip_empty_row_groups(vec![0, 1, 2, 3], None,
&ROW_GROUP_METADATA);
+ assert_eq!(indexes, vec![0, 1, 2, 3]);
+ assert!(result.is_none());
+ }
Review Comment:
Great catch on the reachable producer — this is exactly the regression I was
missing. Added
`test_prepare_strips_row_group_emptied_by_intersecting_selections`, which
drives the `scan_selection` empty-intersection case through `prepare()`. I left
the complementary `scan_selection` normalization (empty intersection → `Skip`,
which would also keep `row_group_indexes()` / metrics / `is_fully_matched`
well-formed) out of this PR to keep it scoped to #24287 — happy to do it as a
small follow-up.
--
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]
