Copilot commented on code in PR #19557:
URL: https://github.com/apache/datafusion/pull/19557#discussion_r2652237048
##########
datafusion/datasource-parquet/src/sort.rs:
##########
@@ -22,6 +22,8 @@ use parquet::arrow::arrow_reader::{RowSelection, RowSelector};
use parquet::file::metadata::ParquetMetaData;
use std::collections::HashMap;
+// datafusion/datasource-parquet/src/sort.rs
+
Review Comment:
This comment appears to be an unnecessary file path reference that was
likely added accidentally during development. It doesn't add any value and
should be removed to keep the code clean.
```suggestion
```
##########
datafusion/datasource-parquet/src/sort.rs:
##########
@@ -388,7 +429,10 @@ mod tests {
RowSelector::skip(100), // RG3
]);
- let reversed = reverse_row_selection(&selection, &metadata).unwrap();
+ let row_groups_to_scan = vec![0, 1, 2];
Review Comment:
This test has a logical inconsistency. The test creates 4 row groups (100
rows each) and the selection covers all 4 row groups (400 rows total), but
`row_groups_to_scan` only includes 3 row groups [0, 1, 2]. This mismatch means
the selection references RG3 (the fourth RowSelector with skip(100)), but RG3
is not in the scan list. This doesn't match the documented behavior where the
row selection should only cover the row groups that are being scanned.
```suggestion
let row_groups_to_scan = vec![0, 1, 2, 3];
```
--
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]
