feat: ORDER BY ALL #15772
feat: ORDER BY ALL #15772
Conversation
github-actions
bot
added
sql
datafusion/expr/src/expr.rs
Outdated
| /// OrderBy Expressions | ||
| pub enum OrderByExprs { | ||
| OrderByExprVec(Vec<OrderByExpr>), | ||
| All { | ||
| exprs: Vec<Expr>, | ||
| options: OrderByOptions, | ||
| }, | ||
| } |
There was a problem hiding this comment.
I'm not sure if we need the Enum.
Maybe it's enough to wrap Vec<OrderByExpr> as a struct and add an extra flag to indicate the ALL
pub struct OrderByExprs {
exprs: Vec<OrderByExpr>,
all: bool
}There was a problem hiding this comment.
This requires converting a datafusion_expr::Expr to a SQLExpr, but I'm not sure if there's a good way to do that.
There was a problem hiding this comment.
Instead of this new enum, can we utilize https://github.com/apache/datafusion-sqlparser-rs/blob/3ec80e187d163c4f90c5bfc7c04ef71a2705a631/src/ast/query.rs#L2222 ?
There was a problem hiding this comment.
this enum seems to only be used in the sql planner, so I don't think it is needed in datafusion-expr -- maybe we can just move this into the datafusion-sql crate and make it pub(crate)
There was a problem hiding this comment.
Okay, I’ll try to move it this weekend
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Added support for
ORDER BY ALLsyntaxAre these changes tested?
order.slttest case from https://duckdb.org/docs/stable/sql/query_syntax/orderby.html#order-by-all-examples
Are there any user-facing changes?