alamb commented on code in PR #25731:
URL: https://github.com/apache/datafusion/pull/25731#discussion_r4158716665
##########
datafusion/datasource-parquet/src/access_plan.rs:
##########
@@ -298,6 +299,39 @@ impl ParquetAccessPlan {
selection: RowSelection,
row_group_meta_data: &[RowGroupMetaData],
) -> Result<Self> {
+ if let Some(mask) = selection.as_mask() {
+ let selection_rows = mask.len();
+ let file_rows = row_group_meta_data
+ .iter()
+ .map(|rg| rg.num_rows() as usize)
+ .sum::<usize>();
+ if selection_rows != file_rows {
+ return exec_err!(
+ "Invalid Parquet RowSelection. File has {file_rows} rows, \
+ but selection specifies {selection_rows} rows."
+ );
+ }
+
+ // Slice the bitmap without materializing selectors. Use
row_count()
Review Comment:
This is similar to what
https://docs.rs/parquet/latest/parquet/arrow/arrow_reader/struct.RowSelection.html#method.split_off
is doing
though then the code says this:
```rust
// Keep this as a single pass over the selector stream rather than
// repeatedly calling `RowSelection::split_off` per row group. The
// `split_off` version is simpler, but it clones/retains
substantially
// more selector buffer capacity for highly fragmented selections.
```
So I am not sure how important avoiding split_off is for masks 🤔
Would you be willing to at least make two different functions to reduce the
indent level
```rust
pub fn try_new_from_overall_row_selection(
selection: RowSelection,
row_group_meta_data: &[RowGroupMetaData],
) -> Result<Self> {
if let Some(mask) = selection.as_mask() {
self.try_new_from_overall_row_selection_mask(mask,
row_group_meta_data)
} else {
self.try_new_from_overall_row_selection_selectors(mask,
row_group_meta_data)
--
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]
