Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion datafusion/core/tests/physical_optimizer/limit_pushdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ use std::sync::Arc;

use crate::physical_optimizer::test_utils::{
coalesce_partitions_exec, global_limit_exec, hash_join_exec, local_limit_exec,
sort_exec, sort_preserving_merge_exec, stream_exec,
parquet_exec_with_sort, sort_exec, sort_preserving_merge_exec,
sort_preserving_merge_exec_with_fetch, stream_exec,
};

use arrow::compute::SortOptions;
use arrow::datatypes::{DataType, Field, Schema, SchemaRef};
use datafusion_common::config::ConfigOptions;
use datafusion_common::error::Result;
use datafusion_datasource::file_scan_config::FileScanConfig;
use datafusion_datasource::source::DataSourceExec;
use datafusion_expr::{JoinType, Operator};
use datafusion_physical_expr::Partitioning;
use datafusion_physical_expr::expressions::{BinaryExpr, col, lit};
Expand Down Expand Up @@ -405,6 +408,34 @@ fn pushes_global_limit_into_multiple_fetch_plans() -> Result<()> {
Ok(())
}

#[test]
fn preserves_order_when_pushing_fetch_from_sort_preserving_merge() -> Result<()> {
let schema = create_schema();
let ordering: LexOrdering = [PhysicalSortExpr {
expr: col("c1", &schema)?,
options: SortOptions::default(),
}]
.into();
let scan = parquet_exec_with_sort(schema, vec![ordering.clone()]);
let local_limit = local_limit_exec(scan, 10);
let plan = sort_preserving_merge_exec_with_fetch(ordering, local_limit, 5);

let optimized = LimitPushdown::new().optimize(plan, &ConfigOptions::new())?;
let scan = optimized.children().swap_remove(0);
let scan = scan
.downcast_ref::<DataSourceExec>()
.expect("fetch should be pushed into the scan");
let config = scan
.data_source()
.downcast_ref::<FileScanConfig>()
.expect("parquet scan should use FileScanConfig");

assert_eq!(config.limit, Some(5));
assert!(config.preserve_order);

Ok(())
}

#[test]
fn keeps_pushed_local_limit_exec_when_there_are_multiple_input_partitions() -> Result<()>
{
Expand Down
6 changes: 4 additions & 2 deletions datafusion/physical-optimizer/src/limit_pushdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ pub fn pushdown_limit_helper(
);
global_state.skip = skip;
global_state.fetch = fetch;
global_state.preserve_order = limit_info.preserve_order;
global_state.preserve_order |= limit_info.preserve_order;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we clear preserve_order when ordered fetch scope ends instead of carrying it into later limits? For example, with SortPreservingMergeExec(fetch=5) -> SortExec -> LocalLimitExec(fetch=10) -> DataSourceExec, SortExec clears outer fetch, but this |= keeps flag set and applies it to independent inner limit. That disables Parquet limit pruning and file-stream work stealing even though inner limit is not order-sensitive. Is this a concern, or am I missing where state is reset?

global_state.satisfied = false;

if let Some(fetch) = fetch
Expand Down Expand Up @@ -196,8 +196,10 @@ pub fn pushdown_limit_helper(
}

// If we have a non-limit operator with fetch capability, update global
// state as necessary:
// state as necessary. A fetched ordered merge selects the leading rows in
// its ordering, so preserve that ordering when pushing the fetch down.
if pushdown_plan.fetch().is_some() {
global_state.preserve_order |= pushdown_plan.is::<SortPreservingMergeExec>();
if global_state.skip == 0 {
global_state.satisfied = true;
}
Expand Down
73 changes: 73 additions & 0 deletions datafusion/sqllogictest/test_files/limit_pruning.slt
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,79 @@ select a from fully_matched_limit where a >= 3 order by a limit 4;
statement ok
drop table fully_matched_limit;

# A fetched SortPreservingMergeExec must preserve scan order when pushing down
# its limit. Otherwise, limit pruning can skip the first partially matched row
# group in favor of the following fully matched row group.
statement ok
CREATE TABLE ordered_limit_source(id INT, passes INT) AS VALUES
(1, 0),
(2, 1),
(3, 1),
(4, 1),
(5, 1),
(6, 1),
(101, 1),
(102, 1),
(103, 1);

query I
COPY (SELECT * FROM ordered_limit_source WHERE id < 100 ORDER BY id)
TO 'test_files/scratch/limit_pruning/ordered_limit/a.parquet'
STORED AS PARQUET
OPTIONS (
'format.max_row_group_size' '3'
);
----
6

query I
COPY (SELECT * FROM ordered_limit_source WHERE id >= 100 ORDER BY id)
TO 'test_files/scratch/limit_pruning/ordered_limit/b.parquet'
STORED AS PARQUET
OPTIONS (
'format.max_row_group_size' '3'
);
----
3

statement ok
drop table ordered_limit_source;

statement ok
set datafusion.execution.target_partitions = 2;

statement ok
CREATE EXTERNAL TABLE ordered_limit(id INT, passes INT)
STORED AS PARQUET
LOCATION 'test_files/scratch/limit_pruning/ordered_limit/'
WITH ORDER (id ASC);

statement ok
set datafusion.explain.physical_plan_only = true;

query TT
explain select * from ordered_limit where passes = 1 order by id limit 3;
----
physical_plan
01)SortPreservingMergeExec: [id@0 ASC NULLS LAST], fetch=3
02)--DataSourceExec: <slt:ignore>limit=3, output_ordering=[id@0 ASC NULLS LAST], file_type=parquet<slt:ignore>

query II
select * from ordered_limit where passes = 1 order by id limit 3;
----
2 1
3 1
4 1

statement ok
drop table ordered_limit;

statement ok
reset datafusion.explain.physical_plan_only;

statement ok
set datafusion.execution.target_partitions = 4;

# limit_pruned_row_groups=0 total → 0 matched
# because of order by, scan needs to preserve sort, so limit pruning is disabled
query TT
Expand Down
Loading