Skip to content

Commit

Permalink
Minor: fix: Include FetchRel when producing LogicalPlan from Sort (ap…
Browse files Browse the repository at this point in the history
…ache#13862)

* include FetchRel when producing LogicalPlan from Sort

* add suggested test

* address review feedback
  • Loading branch information
robtandy authored Dec 21, 2024
1 parent b99400e commit a50ed34
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
38 changes: 31 additions & 7 deletions datafusion/substrait/src/logical_plan/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,21 +361,45 @@ pub fn to_substrait_rel(
}))),
}))
}
LogicalPlan::Sort(sort) => {
let input = to_substrait_rel(sort.input.as_ref(), state, extensions)?;
let sort_fields = sort
.expr
LogicalPlan::Sort(datafusion::logical_expr::Sort { expr, input, fetch }) => {
let sort_fields = expr
.iter()
.map(|e| substrait_sort_field(state, e, sort.input.schema(), extensions))
.map(|e| substrait_sort_field(state, e, input.schema(), extensions))
.collect::<Result<Vec<_>>>()?;
Ok(Box::new(Rel {

let input = to_substrait_rel(input.as_ref(), state, extensions)?;

let sort_rel = Box::new(Rel {
rel_type: Some(RelType::Sort(Box::new(SortRel {
common: None,
input: Some(input),
sorts: sort_fields,
advanced_extension: None,
}))),
}))
});

match fetch {
Some(amount) => {
let count_mode =
Some(fetch_rel::CountMode::CountExpr(Box::new(Expression {
rex_type: Some(RexType::Literal(Literal {
nullable: false,
type_variation_reference: DEFAULT_TYPE_VARIATION_REF,
literal_type: Some(LiteralType::I64(*amount as i64)),
})),
})));
Ok(Box::new(Rel {
rel_type: Some(RelType::Fetch(Box::new(FetchRel {
common: None,
input: Some(sort_rel),
offset_mode: None,
count_mode,
advanced_extension: None,
}))),
}))
}
None => Ok(sort_rel),
}
}
LogicalPlan::Aggregate(agg) => {
let input = to_substrait_rel(agg.input.as_ref(), state, extensions)?;
Expand Down
10 changes: 10 additions & 0 deletions datafusion/substrait/tests/cases/roundtrip_logical_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,16 @@ async fn select_with_filter() -> Result<()> {
roundtrip("SELECT * FROM data WHERE a > 1").await
}

#[tokio::test]
async fn select_with_filter_sort_limit() -> Result<()> {
roundtrip("SELECT * FROM data WHERE a > 1 ORDER BY b ASC LIMIT 2").await
}

#[tokio::test]
async fn select_with_filter_sort_limit_offset() -> Result<()> {
roundtrip("SELECT * FROM data WHERE a > 1 ORDER BY b ASC LIMIT 2 OFFSET 1").await
}

#[tokio::test]
async fn select_with_reused_functions() -> Result<()> {
let ctx = create_context().await?;
Expand Down

0 comments on commit a50ed34

Please sign in to comment.