robtandy commented on code in PR #13862:
URL: https://github.com/apache/datafusion/pull/13862#discussion_r1894410849
##########
datafusion/substrait/tests/cases/roundtrip_logical_plan.rs:
##########
@@ -200,6 +200,11 @@ 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
Review Comment:
Good idea. I'll add.
##########
datafusion/substrait/src/logical_plan/producer.rs:
##########
@@ -368,14 +368,45 @@ pub fn to_substrait_rel(
.iter()
.map(|e| substrait_sort_field(state, e, sort.input.schema(),
extensions))
.collect::<Result<Vec<_>>>()?;
- Ok(Box::new(Rel {
+
+ 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 sort.fetch {
Review Comment:
Making sure I follow:
You mean on line 364 destructuring the plan like
```rust
LogicalPlan::Sort(Sort {expr, input, fetch}) => {...
```
I didn't see that pattern followed in that file for the other LogicalPlan
inners. I think its a good idea though
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
