Skip to content

Commit 46fa442

Browse files
zhuqi-lucasclaude
andcommitted
fix: re-check ordering after statistics-based file sorting
When FileSource returns Unsupported (because validated_output_ordering stripped the ordering due to wrong file order), sort files by statistics and re-check: if files are now non-overlapping and output_ordering is valid, upgrade to Exact (eliminating SortExec). This handles the key case where files have correct within-file ordering (Parquet sorting_columns metadata) but were listed in wrong order (e.g., alphabetical order doesn't match sort key order). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6812ad9 commit 46fa442

2 files changed

Lines changed: 40 additions & 18 deletions

File tree

datafusion/datasource/src/file_scan_config.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,12 +1523,21 @@ impl FileScanConfig {
15231523

15241524
/// Last-resort optimization when FileSource returns `Unsupported`.
15251525
///
1526-
/// Even without within-file ordering guarantees, reordering files by
1527-
/// min/max statistics still helps: TopK queries with dynamic filters
1528-
/// can prune files earlier when files are read in approximate order.
1526+
/// FileSource may return `Unsupported` because `eq_properties` had no
1527+
/// ordering — which happens when `validated_output_ordering()` stripped
1528+
/// the ordering because files were in the wrong order. After sorting
1529+
/// files by statistics, the ordering may become valid again.
15291530
///
1530-
/// Returns `Inexact` (SortExec stays) since we cannot guarantee ordering.
1531-
/// Returns `Unsupported` if no files were actually reordered.
1531+
/// This method:
1532+
/// 1. Sorts files within groups by min/max statistics
1533+
/// 2. Re-checks if the sorted file order makes `output_ordering` valid
1534+
/// 3. If valid AND non-overlapping → `Exact` (SortExec eliminated!)
1535+
/// 4. If files were reordered but ordering not valid → `Inexact`
1536+
/// 5. If no files were reordered → `Unsupported`
1537+
///
1538+
/// This handles the key case where files have correct within-file ordering
1539+
/// (e.g., Parquet sorting_columns metadata) but were listed in wrong order
1540+
/// (e.g., alphabetical order doesn't match sort key order).
15321541
fn try_sort_file_groups_by_statistics(
15331542
&self,
15341543
order: &[PhysicalSortExpr],
@@ -1557,6 +1566,23 @@ impl FileScanConfig {
15571566

15581567
let mut new_config = self.clone();
15591568
new_config.file_groups = result.file_groups;
1569+
1570+
// Re-check: now that files are sorted, does output_ordering become valid?
1571+
// This handles the case where validated_output_ordering() previously
1572+
// stripped the ordering because files were in the wrong order.
1573+
if result.all_non_overlapping && !self.output_ordering.is_empty() {
1574+
// Files are now non-overlapping and we have declared output_ordering.
1575+
// Re-ask the FileSource if this ordering satisfies the request,
1576+
// using eq_properties computed from the NEW (sorted) file groups.
1577+
let new_eq_props = new_config.eq_properties();
1578+
if new_eq_props.ordering_satisfy(order.iter().cloned())? {
1579+
// The sorted file order makes the ordering valid → Exact!
1580+
return Ok(SortOrderPushdownResult::Exact {
1581+
inner: Arc::new(new_config),
1582+
});
1583+
}
1584+
}
1585+
15601586
new_config.output_ordering = vec![];
15611587
Ok(SortOrderPushdownResult::Inexact {
15621588
inner: Arc::new(new_config),

datafusion/sqllogictest/test_files/sort_pushdown.slt

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,16 +1100,15 @@ CREATE EXTERNAL TABLE reversed_parquet(id INT, value INT)
11001100
STORED AS PARQUET
11011101
LOCATION 'test_files/scratch/sort_pushdown/reversed/';
11021102

1103-
# Test 4.1: SortExec must be present because files are not in inter-file order
1103+
# Test 4.1: Files sorted by statistics → non-overlapping → SortExec eliminated
1104+
# (files reordered from [a_high, b_mid, c_low] to [c_low, b_mid, a_high])
11041105
query TT
11051106
EXPLAIN SELECT * FROM reversed_parquet ORDER BY id ASC;
11061107
----
11071108
logical_plan
11081109
01)Sort: reversed_parquet.id ASC NULLS LAST
11091110
02)--TableScan: reversed_parquet projection=[id, value]
1110-
physical_plan
1111-
01)SortExec: expr=[id@0 ASC NULLS LAST], preserve_partitioning=[false]
1112-
02)--DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/c_low.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/b_mid.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/a_high.parquet]]}, projection=[id, value], file_type=parquet
1111+
physical_plan DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/c_low.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/b_mid.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/a_high.parquet]]}, projection=[id, value], output_ordering=[id@0 ASC NULLS LAST], file_type=parquet
11131112

11141113
# Test 4.2: Results must be correct
11151114
query II
@@ -1184,16 +1183,15 @@ STORED AS PARQUET
11841183
LOCATION 'test_files/scratch/sort_pushdown/reversed/'
11851184
WITH ORDER (id ASC);
11861185

1187-
# Test 6.1: SortExec must be present despite WITH ORDER
1186+
# Test 6.1: Files sorted by statistics → non-overlapping → SortExec eliminated
1187+
# WITH ORDER declared + files reordered to correct order
11881188
query TT
11891189
EXPLAIN SELECT * FROM reversed_with_order_parquet ORDER BY id ASC;
11901190
----
11911191
logical_plan
11921192
01)Sort: reversed_with_order_parquet.id ASC NULLS LAST
11931193
02)--TableScan: reversed_with_order_parquet projection=[id, value]
1194-
physical_plan
1195-
01)SortExec: expr=[id@0 ASC NULLS LAST], preserve_partitioning=[false]
1196-
02)--DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/c_low.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/b_mid.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/a_high.parquet]]}, projection=[id, value], file_type=parquet
1194+
physical_plan DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/c_low.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/b_mid.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/reversed/a_high.parquet]]}, projection=[id, value], output_ordering=[id@0 ASC NULLS LAST], file_type=parquet
11971195

11981196
# Test 6.2: Results must be correct
11991197
query II
@@ -1322,17 +1320,15 @@ STORED AS PARQUET
13221320
LOCATION 'test_files/scratch/sort_pushdown/desc_reversed/'
13231321
WITH ORDER (id DESC);
13241322

1325-
# Test 8.1: SortExec must be present — files are in wrong inter-file DESC order
1326-
# (a_low has 1-3, b_high has 7-9; for DESC, b_high should come first)
1323+
# Test 8.1: Files sorted by statistics → non-overlapping → SortExec eliminated
1324+
# (files reordered: b_high(7-9) before a_low(1-3) for DESC order)
13271325
query TT
13281326
EXPLAIN SELECT * FROM desc_reversed_parquet ORDER BY id DESC;
13291327
----
13301328
logical_plan
13311329
01)Sort: desc_reversed_parquet.id DESC NULLS FIRST
13321330
02)--TableScan: desc_reversed_parquet projection=[id, value]
1333-
physical_plan
1334-
01)SortExec: expr=[id@0 DESC], preserve_partitioning=[false]
1335-
02)--DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/desc_reversed/b_high.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/desc_reversed/a_low.parquet]]}, projection=[id, value], file_type=parquet
1331+
physical_plan DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/desc_reversed/b_high.parquet, WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/sort_pushdown/desc_reversed/a_low.parquet]]}, projection=[id, value], output_ordering=[id@0 DESC], file_type=parquet
13361332

13371333
# Test 8.2: Results must be correct
13381334
query II

0 commit comments

Comments
 (0)