feat(planner): support order by in new planner
#5253
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
| -- order by | ||
| SELECT number%3 as c1, number%2 as c2 FROM numbers_mt (10) order by c1 desc, c2 asc; | ||
| SELECT number, null from numbers(3) order by number desc; | ||
| SELECT number%3 as c1, number%2 as c2 FROM numbers_mt (10) order by c1, number desc; |
There was a problem hiding this comment.
Maybe need test SELECT COUNT() FROM numbers_mt(10) GROUP BY number ORDER BY SUM(number);
There was a problem hiding this comment.
SELECT SUM(number) AS s FROM numbers_mt(10) GROUP BY number ORDER BY s;
There was a problem hiding this comment.
Maybe need test
SELECT COUNT() FROM numbers_mt(10) GROUP BY number ORDER BY SUM(number);
Nice, the sort expression(s) can be any expression that would be valid in the query's select list, so I need to process its input schema and produce its output schema. Thank you!
There was a problem hiding this comment.
I haven't figured out how to support aggregate functions in order by, but now that other expressions than aggregate functions are supported, I'll note an issue.
| let expr_name = expr.column_name().clone(); | ||
| if !input_schema.has_field(expr_name.as_str()) { | ||
| let field = if expr.nullable(input_schema)? { | ||
| DataField::new_nullable(expr_name.as_str(), expr.to_data_type(input_schema)?) |
There was a problem hiding this comment.
Just use DataField::new(expr_name.as_str(), expr.to_data_type(input_schema)?)
expr.to_data_type already covered nullable case.
There was a problem hiding this comment.
I actually used new directly at the beginning, but I encountered a bug when I implemented group by the other day, so I still differentiate whether it's nullable here, and I'll sort out the bug tomorrow
|
Let's merge it first? Then I can do other tasks based on the ticket. |
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Support
order byin new plannerChangelog
Related Issues
Fixes #5202