Skip to content

feat: ORDER BY ALL #15772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions datafusion/sql/src/expr/order_by.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
/// If false, interpret numeric literals as constant values.
pub(crate) fn order_by_to_sort_expr(
&self,
exprs: Vec<OrderByExpr>,
order_by_exprs: Vec<OrderByExpr>,
input_schema: &DFSchema,
planner_context: &mut PlannerContext,
literal_to_column: bool,
additional_schema: Option<&DFSchema>,
) -> Result<Vec<SortExpr>> {
if exprs.is_empty() {
if order_by_exprs.is_empty() {
return Ok(vec![]);
}

Expand All @@ -61,13 +61,23 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
None => input_schema,
};

let mut expr_vec = vec![];
for e in exprs {
let mut sort_expr_vec = Vec::with_capacity(order_by_exprs.len());

let make_sort_expr =
|expr: Expr, asc: Option<bool>, nulls_first: Option<bool>| {
let asc = asc.unwrap_or(true);
// When asc is true, by default nulls last to be consistent with postgres
// postgres rule: https://www.postgresql.org/docs/current/queries-order.html
let nulls_first = nulls_first.unwrap_or(!asc);
Sort::new(expr, asc, nulls_first)
};

for order_by_expr in order_by_exprs {
let OrderByExpr {
expr,
options: OrderByOptions { asc, nulls_first },
with_fill,
} = e;
} = order_by_expr;

if let Some(with_fill) = with_fill {
return not_impl_err!("ORDER BY WITH FILL is not supported: {with_fill}");
Expand Down Expand Up @@ -102,15 +112,9 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
self.sql_expr_to_logical_expr(e, order_by_schema, planner_context)?
}
};
let asc = asc.unwrap_or(true);
expr_vec.push(Sort::new(
expr,
asc,
// When asc is true, by default nulls last to be consistent with postgres
// postgres rule: https://www.postgresql.org/docs/current/queries-order.html
nulls_first.unwrap_or(!asc),
))
sort_expr_vec.push(make_sort_expr(expr, asc, nulls_first));
}
Ok(expr_vec)

Ok(sort_expr_vec)
}
}
36 changes: 30 additions & 6 deletions datafusion/sql/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
use crate::stack::StackGuard;
use datafusion_common::{not_impl_err, Constraints, DFSchema, Result};
use datafusion_expr::expr::Sort;
use datafusion_expr::select_expr::SelectExpr;

use datafusion_expr::{
CreateMemoryTable, DdlStatement, Distinct, LogicalPlan, LogicalPlanBuilder,
CreateMemoryTable, DdlStatement, Distinct, Expr, LogicalPlan, LogicalPlanBuilder,
};
use sqlparser::ast::{
Expr as SQLExpr, Offset as SQLOffset, OrderBy, OrderByExpr, OrderByKind, Query,
SelectInto, SetExpr,
Expr as SQLExpr, Ident, Offset as SQLOffset, OrderBy, OrderByExpr, OrderByKind,
Query, SelectInto, SetExpr,
};
use sqlparser::tokenizer::Span;

impl<S: ContextProvider> SqlToRel<'_, S> {
/// Generate a logical plan from an SQL query/subquery
Expand Down Expand Up @@ -158,7 +159,7 @@ fn to_order_by_exprs(order_by: Option<OrderBy>) -> Result<Vec<OrderByExpr>> {
/// Returns the order by expressions from the query with the select expressions.
pub(crate) fn to_order_by_exprs_with_select(
order_by: Option<OrderBy>,
_select_exprs: Option<&Vec<SelectExpr>>, // TODO: ORDER BY ALL
select_exprs: Option<&Vec<Expr>>,
) -> Result<Vec<OrderByExpr>> {
let Some(OrderBy { kind, interpolate }) = order_by else {
// If no order by, return an empty array.
Expand All @@ -168,7 +169,30 @@ pub(crate) fn to_order_by_exprs_with_select(
return not_impl_err!("ORDER BY INTERPOLATE is not supported");
}
match kind {
OrderByKind::All(_) => not_impl_err!("ORDER BY ALL is not supported"),
OrderByKind::All(order_by_options) => {
let Some(exprs) = select_exprs else {
return Ok(vec![]);
};
let order_by_exprs = exprs
.iter()
.map(|select_expr| match select_expr {
Expr::Column(column) => Ok(OrderByExpr {
expr: SQLExpr::Identifier(Ident {
value: column.name.clone(),
quote_style: None,
span: Span::empty(),
}),
options: order_by_options.clone(),
with_fill: None,
}),
// TODO: Support other types of expressions
_ => not_impl_err!(
"ORDER BY ALL is not supported for non-column expressions"
),
})
.collect::<Result<Vec<_>>>()?;
Ok(order_by_exprs)
}
OrderByKind::Expressions(order_by_exprs) => Ok(order_by_exprs),
}
}
6 changes: 3 additions & 3 deletions datafusion/sql/src/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
planner_context,
)?;

let order_by =
to_order_by_exprs_with_select(query_order_by, Some(&select_exprs))?;

// Having and group by clause may reference aliases defined in select projection
let projected_plan = self.project(base_plan.clone(), select_exprs)?;
let select_exprs = projected_plan.expressions();

let order_by =
to_order_by_exprs_with_select(query_order_by, Some(&select_exprs))?;

// Place the fields of the base plan at the front so that when there are references
// with the same name, the fields of the base plan will be searched first.
// See https://github.com/apache/datafusion/issues/9162
Expand Down
39 changes: 39 additions & 0 deletions datafusion/sqllogictest/test_files/order.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1380,3 +1380,42 @@ physical_plan

statement ok
drop table table_with_ordered_not_null;

# ORDER BY ALL
statement ok
set datafusion.sql_parser.dialect = 'DuckDB';

statement ok
CREATE OR REPLACE TABLE addresses AS
SELECT '123 Quack Blvd' AS address, 'DuckTown' AS city, '11111' AS zip
UNION ALL
SELECT '111 Duck Duck Goose Ln', 'DuckTown', '11111'
UNION ALL
SELECT '111 Duck Duck Goose Ln', 'Duck Town', '11111'
UNION ALL
SELECT '111 Duck Duck Goose Ln', 'Duck Town', '11111-0001';


query TTT
SELECT * FROM addresses ORDER BY ALL;
----
111 Duck Duck Goose Ln Duck Town 11111
111 Duck Duck Goose Ln Duck Town 11111-0001
111 Duck Duck Goose Ln DuckTown 11111
123 Quack Blvd DuckTown 11111

query TTT
SELECT * FROM addresses ORDER BY ALL DESC;
----
123 Quack Blvd DuckTown 11111
111 Duck Duck Goose Ln DuckTown 11111
111 Duck Duck Goose Ln Duck Town 11111-0001
111 Duck Duck Goose Ln Duck Town 11111

query TT
SELECT address, zip FROM addresses ORDER BY ALL;
----
111 Duck Duck Goose Ln 11111
111 Duck Duck Goose Ln 11111
111 Duck Duck Goose Ln 11111-0001
123 Quack Blvd 11111