Skip to content

Commit 50fb25e

Browse files
zhuqi-lucasclaude
andcommitted
address review: remove unnecessary stats computation in reverse path, improve docs
- Remove dead stats computation in reverse_file_groups branch (reverse path is always Inexact, so all_non_overlapping is unused) - Add reverse prefix matching documentation to pushdown_sort module Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4646b2b commit 50fb25e

2 files changed

Lines changed: 8 additions & 19 deletions

File tree

datafusion/datasource/src/file_scan_config.rs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,24 +1405,11 @@ impl FileScanConfig {
14051405
false
14061406
}
14071407
} else {
1408-
// When reversing, check if reversed groups are non-overlapping
1409-
if let Some(sort_order) = LexOrdering::new(order.iter().cloned()) {
1410-
let projected_schema = new_config.projected_schema()?;
1411-
let projection_indices = new_config
1412-
.file_source
1413-
.projection()
1414-
.as_ref()
1415-
.and_then(|p| ordered_column_indices_from_projection(p));
1416-
let result = Self::sort_files_within_groups_by_statistics(
1417-
&new_config.file_groups,
1418-
&sort_order,
1419-
&projected_schema,
1420-
projection_indices.as_deref(),
1421-
);
1422-
result.all_non_overlapping
1423-
} else {
1424-
false
1425-
}
1408+
// When reversing, files are already reversed above. We skip
1409+
// statistics-based sorting here because it would undo the reversal.
1410+
// Note: reverse path is always Inexact, so all_non_overlapping
1411+
// is not used (is_exact is false).
1412+
false
14261413
};
14271414

14281415
if is_exact && all_non_overlapping {

datafusion/physical-optimizer/src/pushdown_sort.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@
4747
//! - **Non-overlapping detection**: when files have non-overlapping ranges and matching
4848
//! within-file ordering, the combined scan is `Exact` (sort eliminated)
4949
//! - **Prefix matching**: if data has ordering [A DESC, B ASC] and query needs
50-
//! [A DESC], the existing ordering satisfies the requirement
50+
//! [A DESC], the existing ordering satisfies the requirement (`Exact`).
51+
//! If the query needs [A ASC] (reverse of the prefix), a reverse scan is
52+
//! used (`Inexact`, `SortExec` retained)
5153
//!
5254
//! Related issue: <https://github.com/apache/datafusion/issues/17348>
5355

0 commit comments

Comments
 (0)