Skip to content

Commit 74e4974

Browse files
committed
initial work
Signed-off-by: Adam Gutglick <adamgsal@gmail.com>
1 parent b0b5471 commit 74e4974

3 files changed

Lines changed: 39 additions & 17 deletions

File tree

‎datafusion/core/tests/physical_optimizer/ensure_requirements.rs‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,15 +1536,10 @@ fn test_collect_left_join_keeps_hash_partitioned_build_side_coalesce() -> Result
15361536
// Limits with a `skip`
15371537
// ========================================================================
15381538

1539-
/// A sort pushed below `GlobalLimitExec` with a non-zero `skip` must ask its
1540-
/// input for `skip + fetch` rows; asking for only `fetch` rows used to leave
1541-
/// `LIMIT 10 OFFSET 5` with 5 result rows.
1542-
///
1543-
/// This checks a single pass only: the sort is inserted by `pushdown_sorts`,
1544-
/// which runs after `parallelize_sorts`, so a second pass would additionally
1545-
/// parallelize it into `SortPreservingMergeExec` + partitioned `SortExec`.
1539+
/// Keep sorting above an unordered limit so its input can stop early and only
1540+
/// the rows retained after the offset need sorting.
15461541
#[test]
1547-
fn test_sort_pushed_below_limit_with_skip_keeps_skip_rows() -> Result<()> {
1542+
fn test_keep_sort_above_unordered_limit_with_skip() -> Result<()> {
15481543
let source = Arc::new(MockMultiPartitionExec::new(4));
15491544
let coalesce = Arc::new(CoalescePartitionsExec::new(source));
15501545
let limit = Arc::new(GlobalLimitExec::new(coalesce, 5, Some(10)));
@@ -1553,10 +1548,36 @@ fn test_sort_pushed_below_limit_with_skip_keeps_skip_rows() -> Result<()> {
15531548

15541549
let optimized = optimize_and_sanity_check(sort)?;
15551550
assert_snapshot!(plan_string(&optimized), @r"
1556-
GlobalLimitExec: skip=5, fetch=10
1557-
SortExec: TopK(fetch=15), expr=[a@0 DESC], preserve_partitioning=[false]
1551+
SortExec: expr=[a@0 DESC], preserve_partitioning=[false]
1552+
GlobalLimitExec: skip=5, fetch=10
15581553
CoalescePartitionsExec
15591554
MockMultiPartitionExec
15601555
");
15611556
Ok(())
15621557
}
1558+
1559+
/// Refining an existing ordering can still push a TopK below the limit. It must
1560+
/// retain `skip + fetch` rows so the offset does not reduce the result count.
1561+
#[test]
1562+
fn test_sort_pushed_below_ordered_limit_with_skip_keeps_skip_rows() -> Result<()> {
1563+
let source = Arc::new(MockMultiPartitionExec::new(4));
1564+
let mut ordering = sort_expr_on("a", 0, false, false);
1565+
let ordered_input = Arc::new(MockReqExec::new(
1566+
source,
1567+
Distribution::SinglePartition,
1568+
Some(ordering.clone()),
1569+
));
1570+
let limit = Arc::new(GlobalLimitExec::new(ordered_input, 5, Some(10)));
1571+
ordering.extend(sort_expr_on("b", 1, false, false));
1572+
let sort: Arc<dyn ExecutionPlan> = Arc::new(SortExec::new(ordering, limit));
1573+
1574+
let optimized = optimize_and_sanity_check(sort)?;
1575+
assert_snapshot!(plan_string(&optimized), @r"
1576+
GlobalLimitExec: skip=5, fetch=10
1577+
SortExec: TopK(fetch=15), expr=[a@0 ASC NULLS LAST, b@1 ASC NULLS LAST], preserve_partitioning=[false], sort_prefix=[a@0 ASC NULLS LAST]
1578+
MockReqExec
1579+
SortPreservingMergeExec: [a@0 ASC NULLS LAST]
1580+
MockMultiPartitionExec
1581+
");
1582+
Ok(())
1583+
}

‎datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,10 @@ fn pushdown_requirement_to_children(
566566
} else {
567567
parent_required.clone()
568568
};
569+
570+
// Keep sorting above an unordered limit so the input can stop early.
569571
let Some(ordering) = plan.properties().output_ordering() else {
570-
return Ok(Some(vec![Some(child_required)]));
572+
return Ok(None);
571573
};
572574
if plan.properties().eq_properties.requirements_compatible(
573575
parent_required.first().clone(),

‎datafusion/sqllogictest/test_files/limit.slt‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,8 @@ SELECT COUNT(*) FROM (SELECT a FROM t1 LIMIT 3 OFFSET 8);
339339
----
340340
2
341341

342-
# A sort above LIMIT ... OFFSET is pushed below the limit as a TopK. The TopK
343-
# has to keep `skip + fetch` rows (3 + 4 = 7) so that the limit can still skip
344-
# 3 rows and return 4; keeping only `fetch` rows returned a single row.
342+
# Keep the sort above an unordered LIMIT ... OFFSET so the scan can stop
343+
# after `skip + fetch` rows (3 + 4 = 7), and only the 4 selected rows are sorted.
345344
query TT
346345
EXPLAIN SELECT * FROM (SELECT a FROM t1 LIMIT 4 OFFSET 3) ORDER BY a;
347346
----
@@ -350,9 +349,9 @@ logical_plan
350349
02)--Limit: skip=3, fetch=4
351350
03)----TableScan: t1 projection=[a], fetch=7
352351
physical_plan
353-
01)GlobalLimitExec: skip=3, fetch=4
354-
02)--SortExec: TopK(fetch=7), expr=[a@0 ASC NULLS LAST], preserve_partitioning=[false]
355-
03)----DataSourceExec: partitions=1, partition_sizes=[1]
352+
01)SortExec: expr=[a@0 ASC NULLS LAST], preserve_partitioning=[false]
353+
02)--GlobalLimitExec: skip=3, fetch=4
354+
03)----DataSourceExec: partitions=1, partition_sizes=[1], fetch=7
356355

357356
query I
358357
SELECT * FROM (SELECT a FROM t1 LIMIT 4 OFFSET 3) ORDER BY a;

0 commit comments

Comments
 (0)