Skip to content

Commit 140b7cb

Browse files
test: cover ordered selection in custom sort pushdown
1 parent a36fdc5 commit 140b7cb

3 files changed

Lines changed: 82 additions & 13 deletions

File tree

‎datafusion/sqllogictest/src/test_context.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use datafusion::logical_expr::{
4646
ColumnarValue, Expr, ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl, Signature,
4747
Volatility, create_udf,
4848
};
49+
use datafusion::physical_optimizer::optimizer::PhysicalOptimizer;
4950
use datafusion::physical_plan::ExecutionPlan;
5051
use datafusion::prelude::*;
5152
use datafusion::{
@@ -114,6 +115,7 @@ impl TestContext {
114115
/// If `None` is returned (e.g. because some needed feature is not
115116
/// enabled), the file should be skipped
116117
pub async fn try_new_for_test_file(relative_path: &Path) -> Option<Self> {
118+
let file_name = relative_path.file_name().unwrap().to_str().unwrap();
117119
let config = SessionConfig::new()
118120
// hardcode target partitions so plans are deterministic
119121
.with_target_partitions(4);
@@ -177,11 +179,23 @@ impl TestContext {
177179
state_builder = state_builder.with_statistics_registry(registry);
178180
}
179181

182+
if file_name == "sort_pushdown.slt" {
183+
let mut rules = PhysicalOptimizer::new().rules;
184+
let ensure_requirements = rules
185+
.iter()
186+
.position(|rule| rule.name() == "EnsureRequirements")
187+
.expect("default optimizer includes EnsureRequirements");
188+
rules.insert(
189+
ensure_requirements,
190+
Arc::new(custom_sort_pushdown::CustomLimitRule),
191+
);
192+
state_builder = state_builder.with_physical_optimizer_rules(rules);
193+
}
194+
180195
let state = state_builder.build();
181196

182197
let mut test_ctx = TestContext::new(SessionContext::new_with_state(state));
183198

184-
let file_name = relative_path.file_name().unwrap().to_str().unwrap();
185199
match file_name {
186200
"sort_pushdown.slt" => {
187201
custom_sort_pushdown::register_custom_sort_pushdown(

‎datafusion/sqllogictest/src/test_context/custom_sort_pushdown.rs‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,22 @@ use arrow::datatypes::{self as arrow_schema, SchemaRef};
2323
use async_trait::async_trait;
2424
use datafusion::catalog::{Session, TableProvider};
2525
use datafusion::common::Result;
26-
use datafusion::common::tree_node::TreeNodeRecursion;
26+
use datafusion::common::config::ConfigOptions;
27+
use datafusion::common::tree_node::{Transformed, TreeNode, TreeNodeRecursion};
2728
use datafusion::execution::{SendableRecordBatchStream, TaskContext};
2829
use datafusion::logical_expr::{Expr, Operator, TableType};
2930
use datafusion::physical_expr::expressions::{binary, col, lit, negative};
3031
use datafusion::physical_expr::projection::ProjectionExprs;
3132
use datafusion::physical_expr::{PhysicalExpr, PhysicalSortExpr};
33+
use datafusion::physical_optimizer::PhysicalOptimizerRule;
3234
use datafusion::physical_plan::filter::FilterExec;
3335
use datafusion::physical_plan::limit::LocalLimitExec;
3436
use datafusion::physical_plan::projection::ProjectionExec;
37+
use datafusion::physical_plan::sorts::sort::SortExec;
3538
use datafusion::physical_plan::test::TestMemoryExec;
3639
use datafusion::physical_plan::{
37-
ChildrenPropertiesMode, DisplayAs, DisplayFormatType, ExecutionPlan, PlanProperties,
38-
ReplaceChildrenOptions,
40+
ChildrenPropertiesMode, DisplayAs, DisplayFormatType, ExecutionPlan,
41+
ExecutionPlanProperties, PlanProperties, ReplaceChildrenOptions,
3942
};
4043
use datafusion::prelude::SessionContext;
4144

@@ -79,18 +82,65 @@ pub(super) fn register_custom_sort_pushdown(ctx: &SessionContext) -> Result<()>
7982

8083
let batch = record_batch!(("a", Int32, [1, 2, 3]), ("b", Int32, [30, 20, 10]))?;
8184
let schema = batch.schema();
82-
let ordering = [PhysicalSortExpr::new_default(col("a", &schema)?)].into();
85+
let ordering =
86+
[PhysicalSortExpr::new_default(col("a", &schema)?).nulls_last()].into();
8387
let input = Arc::new(
8488
TestMemoryExec::try_new(&[vec![batch]], Arc::clone(&schema), None)?
8589
.try_with_sort_information(vec![ordering])?,
8690
);
87-
ctx.register_table(
88-
"custom_limited_input",
89-
Arc::new(CustomExec(Arc::new(LocalLimitExec::new(input, 2)))),
90-
)?;
91+
ctx.register_table("custom_ordered_input", Arc::new(CustomExec(input)))?;
9192
Ok(())
9293
}
9394

95+
/// Exposes the SQL fixture's ordered selection as a custom limit before sort pushdown.
96+
#[derive(Debug)]
97+
pub(super) struct CustomLimitRule;
98+
99+
impl PhysicalOptimizerRule for CustomLimitRule {
100+
fn optimize(
101+
&self,
102+
plan: Arc<dyn ExecutionPlan>,
103+
_: &ConfigOptions,
104+
) -> Result<Arc<dyn ExecutionPlan>> {
105+
plan.transform_up(|plan| {
106+
let Some(sort) = plan.downcast_ref::<SortExec>() else {
107+
return Ok(Transformed::no(plan));
108+
};
109+
let Some(fetch) = sort.fetch() else {
110+
return Ok(Transformed::no(plan));
111+
};
112+
let input = sort.input();
113+
let source = input
114+
.downcast_ref::<ProjectionExec>()
115+
.map_or(input, |projection| projection.input());
116+
let Some(source) = source.downcast_ref::<CustomExec>() else {
117+
return Ok(Transformed::no(plan));
118+
};
119+
// Only the dedicated ordered source is a wrapped memory scan.
120+
if !source.0.is::<TestMemoryExec>()
121+
|| input.output_partitioning().partition_count() != 1
122+
|| !input
123+
.equivalence_properties()
124+
.ordering_satisfy(sort.expr().clone())?
125+
{
126+
return Ok(Transformed::no(plan));
127+
}
128+
let limit = LocalLimitExec::new(Arc::clone(input), fetch);
129+
let plan: Arc<dyn ExecutionPlan> = Arc::new(CustomExec(Arc::new(limit)));
130+
Ok(Transformed::yes(plan))
131+
})
132+
.map(|transformed| transformed.data)
133+
}
134+
135+
fn name(&self) -> &str {
136+
"CustomLimitRule"
137+
}
138+
139+
fn schema_check(&self) -> bool {
140+
true
141+
}
142+
}
143+
94144
/// Exposes a native plan's children and properties under a distinct plan type,
95145
/// exercising the generic custom-plan sort pushdown path.
96146
#[derive(Debug, Clone)]

‎datafusion/sqllogictest/test_files/sort_pushdown.slt‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,26 @@ physical_plan
7373
03)----CooperativeExec
7474
04)------DataSourceExec: partitions=1, partition_sizes=[1]
7575

76-
# The input is ordered by a. Sorting by b must keep the first two selected rows.
76+
# The inner ORDER BY determines which rows LIMIT selects; the outer sort only
77+
# changes their display order. The fixture plans that selection as a custom limit.
7778
query II
78-
SELECT a, b FROM custom_limited_input ORDER BY b;
79+
SELECT a, b
80+
FROM (SELECT a, b FROM custom_ordered_input ORDER BY a LIMIT 2) s
81+
ORDER BY b;
7982
----
8083
2 20
8184
1 30
8285

8386
query TT
84-
EXPLAIN SELECT a, b FROM custom_limited_input ORDER BY b;
87+
EXPLAIN SELECT a, b
88+
FROM (SELECT a, b FROM custom_ordered_input ORDER BY a LIMIT 2) s
89+
ORDER BY b;
8590
----
8691
physical_plan
8792
01)SortExec: expr=[b@1 ASC NULLS LAST], preserve_partitioning=[false]
8893
02)--CustomLocalLimitExec: fetch=2
8994
03)----CooperativeExec
90-
04)------DataSourceExec: partitions=1, partition_sizes=[1], output_ordering=a@0 ASC
95+
04)------CustomDataSourceExec: partitions=1, partition_sizes=[1], output_ordering=a@0 ASC NULLS LAST
9196

9297
statement ok
9398
SET datafusion.explain.physical_plan_only = false;

0 commit comments

Comments
 (0)