Skip to content

Commit 96638f8

Browse files
author
silezhou
committed
feat: ORDER BY ALL
1 parent 6ffed55 commit 96638f8

File tree

7 files changed

+173
-70
lines changed

7 files changed

+173
-70
lines changed

datafusion/expr/src/expr.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use datafusion_common::{
3939
use datafusion_functions_window_common::field::WindowUDFFieldArgs;
4040
use sqlparser::ast::{
4141
display_comma_separated, ExceptSelectItem, ExcludeSelectItem, IlikeSelectItem,
42-
NullTreatment, RenameSelectItem, ReplaceSelectElement,
42+
NullTreatment, OrderByExpr, OrderByOptions, RenameSelectItem, ReplaceSelectElement,
4343
};
4444

4545
/// Represents logical expressions such as `A + 1`, or `CAST(c1 AS int)`.
@@ -701,6 +701,24 @@ impl TryCast {
701701
}
702702
}
703703

704+
/// OrderBy Expressions
705+
pub enum OrderByExprs {
706+
OrderByExprVec(Vec<OrderByExpr>),
707+
All {
708+
exprs: Vec<Expr>,
709+
options: OrderByOptions,
710+
},
711+
}
712+
713+
impl OrderByExprs {
714+
pub fn is_empty(&self) -> bool {
715+
match self {
716+
OrderByExprs::OrderByExprVec(exprs) => exprs.is_empty(),
717+
OrderByExprs::All { exprs, .. } => exprs.is_empty(),
718+
}
719+
}
720+
}
721+
704722
/// SORT expression
705723
#[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
706724
pub struct Sort {

datafusion/sql/src/expr/function.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use datafusion_common::{
2222
internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err,
2323
DFSchema, Dependency, Diagnostic, Result, Span,
2424
};
25-
use datafusion_expr::expr::{ScalarFunction, Unnest, WildcardOptions};
25+
use datafusion_expr::expr::{OrderByExprs, ScalarFunction, Unnest, WildcardOptions};
2626
use datafusion_expr::planner::{PlannerResult, RawAggregateExpr, RawWindowExpr};
2727
use datafusion_expr::{
2828
expr, Expr, ExprFunctionExt, ExprSchemable, WindowFrame, WindowFunctionDefinition,
@@ -276,7 +276,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
276276
.map(|e| self.sql_expr_to_logical_expr(e, schema, planner_context))
277277
.collect::<Result<Vec<_>>>()?;
278278
let mut order_by = self.order_by_to_sort_expr(
279-
window.order_by,
279+
OrderByExprs::OrderByExprVec(window.order_by),
280280
schema,
281281
planner_context,
282282
// Numeric literals in window function ORDER BY are treated as constants
@@ -357,7 +357,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
357357
// User defined aggregate functions (UDAF) have precedence in case it has the same name as a scalar built-in function
358358
if let Some(fm) = self.context_provider.get_aggregate_meta(&name) {
359359
let order_by = self.order_by_to_sort_expr(
360-
order_by,
360+
OrderByExprs::OrderByExprVec(order_by),
361361
schema,
362362
planner_context,
363363
true,

datafusion/sql/src/expr/order_by.rs

Lines changed: 67 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
1919
use datafusion_common::{
2020
not_impl_err, plan_datafusion_err, plan_err, Column, DFSchema, Result,
2121
};
22-
use datafusion_expr::expr::Sort;
22+
use datafusion_expr::expr::{OrderByExprs, Sort};
2323
use datafusion_expr::{Expr, SortExpr};
2424
use sqlparser::ast::{
2525
Expr as SQLExpr, OrderByExpr, OrderByOptions, Value, ValueWithSpan,
@@ -41,16 +41,12 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
4141
/// If false, interpret numeric literals as constant values.
4242
pub(crate) fn order_by_to_sort_expr(
4343
&self,
44-
exprs: Vec<OrderByExpr>,
44+
order_by_exprs: OrderByExprs,
4545
input_schema: &DFSchema,
4646
planner_context: &mut PlannerContext,
4747
literal_to_column: bool,
4848
additional_schema: Option<&DFSchema>,
4949
) -> Result<Vec<SortExpr>> {
50-
if exprs.is_empty() {
51-
return Ok(vec![]);
52-
}
53-
5450
let mut combined_schema;
5551
let order_by_schema = match additional_schema {
5652
Some(schema) => {
@@ -61,56 +57,78 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
6157
None => input_schema,
6258
};
6359

64-
let mut expr_vec = vec![];
65-
for e in exprs {
66-
let OrderByExpr {
67-
expr,
68-
options: OrderByOptions { asc, nulls_first },
69-
with_fill,
70-
} = e;
60+
if order_by_exprs.is_empty() {
61+
return Ok(vec![]);
62+
}
7163

72-
if let Some(with_fill) = with_fill {
73-
return not_impl_err!("ORDER BY WITH FILL is not supported: {with_fill}");
74-
}
64+
let mut sort_expr_vec = vec![];
7565

76-
let expr = match expr {
77-
SQLExpr::Value(ValueWithSpan {
78-
value: Value::Number(v, _),
79-
span: _,
80-
}) if literal_to_column => {
81-
let field_index = v
82-
.parse::<usize>()
83-
.map_err(|err| plan_datafusion_err!("{}", err))?;
66+
let make_sort_expr =
67+
|expr: Expr, asc: Option<bool>, nulls_first: Option<bool>| {
68+
let asc = asc.unwrap_or(true);
69+
// When asc is true, by default nulls last to be consistent with postgres
70+
// postgres rule: https://www.postgresql.org/docs/current/queries-order.html
71+
let nulls_first = nulls_first.unwrap_or(!asc);
72+
Sort::new(expr, asc, nulls_first)
73+
};
8474

85-
if field_index == 0 {
86-
return plan_err!(
87-
"Order by index starts at 1 for column indexes"
88-
);
89-
} else if input_schema.fields().len() < field_index {
90-
return plan_err!(
91-
"Order by column out of bounds, specified: {}, max: {}",
92-
field_index,
93-
input_schema.fields().len()
75+
match order_by_exprs {
76+
OrderByExprs::OrderByExprVec(expressions) => {
77+
for e in expressions {
78+
let OrderByExpr {
79+
expr,
80+
options: OrderByOptions { asc, nulls_first },
81+
with_fill,
82+
} = e;
83+
84+
if let Some(with_fill) = with_fill {
85+
return not_impl_err!(
86+
"ORDER BY WITH FILL is not supported: {with_fill}"
9487
);
9588
}
9689

97-
Expr::Column(Column::from(
98-
input_schema.qualified_field(field_index - 1),
99-
))
90+
let expr = match expr {
91+
SQLExpr::Value(ValueWithSpan {
92+
value: Value::Number(v, _),
93+
span: _,
94+
}) if literal_to_column => {
95+
let field_index = v
96+
.parse::<usize>()
97+
.map_err(|err| plan_datafusion_err!("{}", err))?;
98+
99+
if field_index == 0 {
100+
return plan_err!(
101+
"Order by index starts at 1 for column indexes"
102+
);
103+
} else if input_schema.fields().len() < field_index {
104+
return plan_err!(
105+
"Order by column out of bounds, specified: {}, max: {}",
106+
field_index,
107+
input_schema.fields().len()
108+
);
109+
}
110+
111+
Expr::Column(Column::from(
112+
input_schema.qualified_field(field_index - 1),
113+
))
114+
}
115+
e => self.sql_expr_to_logical_expr(
116+
e,
117+
order_by_schema,
118+
planner_context,
119+
)?,
120+
};
121+
sort_expr_vec.push(make_sort_expr(expr, asc, nulls_first));
100122
}
101-
e => {
102-
self.sql_expr_to_logical_expr(e, order_by_schema, planner_context)?
123+
}
124+
OrderByExprs::All { exprs, options } => {
125+
let OrderByOptions { asc, nulls_first } = options;
126+
for expr in exprs {
127+
sort_expr_vec.push(make_sort_expr(expr, asc, nulls_first));
103128
}
104-
};
105-
let asc = asc.unwrap_or(true);
106-
expr_vec.push(Sort::new(
107-
expr,
108-
asc,
109-
// When asc is true, by default nulls last to be consistent with postgres
110-
// postgres rule: https://www.postgresql.org/docs/current/queries-order.html
111-
nulls_first.unwrap_or(!asc),
112-
))
113-
}
114-
Ok(expr_vec)
129+
}
130+
};
131+
132+
Ok(sort_expr_vec)
115133
}
116134
}

datafusion/sql/src/query.rs

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
2121

2222
use crate::stack::StackGuard;
2323
use datafusion_common::{not_impl_err, Constraints, DFSchema, Result};
24-
use datafusion_expr::expr::Sort;
25-
use datafusion_expr::select_expr::SelectExpr;
24+
use datafusion_expr::expr::{OrderByExprs, Sort};
25+
2626
use datafusion_expr::{
27-
CreateMemoryTable, DdlStatement, Distinct, LogicalPlan, LogicalPlanBuilder,
27+
CreateMemoryTable, DdlStatement, Distinct, Expr, LogicalPlan, LogicalPlanBuilder,
2828
};
2929
use sqlparser::ast::{
30-
Expr as SQLExpr, Offset as SQLOffset, OrderBy, OrderByExpr, OrderByKind, Query,
31-
SelectInto, SetExpr,
30+
Expr as SQLExpr, Offset as SQLOffset, OrderBy, OrderByKind, Query, SelectInto,
31+
SetExpr,
3232
};
3333

3434
impl<S: ContextProvider> SqlToRel<'_, S> {
@@ -151,24 +151,46 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
151151
}
152152

153153
/// Returns the order by expressions from the query.
154-
fn to_order_by_exprs(order_by: Option<OrderBy>) -> Result<Vec<OrderByExpr>> {
154+
fn to_order_by_exprs(order_by: Option<OrderBy>) -> Result<OrderByExprs> {
155155
to_order_by_exprs_with_select(order_by, None)
156156
}
157157

158158
/// Returns the order by expressions from the query with the select expressions.
159159
pub(crate) fn to_order_by_exprs_with_select(
160160
order_by: Option<OrderBy>,
161-
_select_exprs: Option<&Vec<SelectExpr>>, // TODO: ORDER BY ALL
162-
) -> Result<Vec<OrderByExpr>> {
161+
select_exprs: Option<&Vec<Expr>>,
162+
) -> Result<OrderByExprs> {
163163
let Some(OrderBy { kind, interpolate }) = order_by else {
164164
// If no order by, return an empty array.
165-
return Ok(vec![]);
165+
return Ok(OrderByExprs::OrderByExprVec(vec![]));
166166
};
167167
if let Some(_interpolate) = interpolate {
168168
return not_impl_err!("ORDER BY INTERPOLATE is not supported");
169169
}
170170
match kind {
171-
OrderByKind::All(_) => not_impl_err!("ORDER BY ALL is not supported"),
172-
OrderByKind::Expressions(order_by_exprs) => Ok(order_by_exprs),
171+
OrderByKind::All(order_by_options) => {
172+
let Some(exprs) = select_exprs else {
173+
return Ok(OrderByExprs::All {
174+
exprs: vec![],
175+
options: order_by_options,
176+
});
177+
};
178+
179+
let order_by_epxrs = exprs
180+
.iter()
181+
.filter_map(|select_expr| match select_expr {
182+
Expr::Column(_) => Some(select_expr.clone()),
183+
_ => None,
184+
})
185+
.collect::<Vec<_>>();
186+
187+
Ok(OrderByExprs::All {
188+
exprs: order_by_epxrs,
189+
options: order_by_options,
190+
})
191+
}
192+
OrderByKind::Expressions(order_by_exprs) => {
193+
Ok(OrderByExprs::OrderByExprVec(order_by_exprs))
194+
}
173195
}
174196
}

datafusion/sql/src/select.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
9494
planner_context,
9595
)?;
9696

97-
let order_by =
98-
to_order_by_exprs_with_select(query_order_by, Some(&select_exprs))?;
99-
10097
// Having and group by clause may reference aliases defined in select projection
10198
let projected_plan = self.project(base_plan.clone(), select_exprs)?;
10299
let select_exprs = projected_plan.expressions();
103100

101+
let order_by =
102+
to_order_by_exprs_with_select(query_order_by, Some(&select_exprs))?;
103+
104104
// Place the fields of the base plan at the front so that when there are references
105105
// with the same name, the fields of the base plan will be searched first.
106106
// See https://github.com/apache/datafusion/issues/9162

datafusion/sql/src/statement.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use datafusion_common::{
3939
ToDFSchema,
4040
};
4141
use datafusion_expr::dml::{CopyTo, InsertOp};
42+
use datafusion_expr::expr::OrderByExprs;
4243
use datafusion_expr::expr_rewriter::normalize_col_with_schemas_and_ambiguity_check;
4344
use datafusion_expr::logical_plan::builder::project;
4445
use datafusion_expr::logical_plan::DdlStatement;
@@ -1240,7 +1241,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
12401241
.to_dfschema_ref()?;
12411242
let using: Option<String> = using.as_ref().map(ident_to_string);
12421243
let columns = self.order_by_to_sort_expr(
1243-
columns,
1244+
OrderByExprs::OrderByExprVec(columns),
12441245
&table_schema,
12451246
planner_context,
12461247
false,
@@ -1423,8 +1424,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
14231424
let mut all_results = vec![];
14241425
for expr in order_exprs {
14251426
// Convert each OrderByExpr to a SortExpr:
1426-
let expr_vec =
1427-
self.order_by_to_sort_expr(expr, schema, planner_context, true, None)?;
1427+
let expr_vec = self.order_by_to_sort_expr(
1428+
OrderByExprs::OrderByExprVec(expr),
1429+
schema,
1430+
planner_context,
1431+
true,
1432+
None,
1433+
)?;
14281434
// Verify that columns of all SortExprs exist in the schema:
14291435
for sort in expr_vec.iter() {
14301436
for column in sort.expr.column_refs().iter() {

datafusion/sqllogictest/test_files/order.slt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,3 +1380,42 @@ physical_plan
13801380

13811381
statement ok
13821382
drop table table_with_ordered_not_null;
1383+
1384+
# ORDER BY ALL
1385+
statement ok
1386+
set datafusion.sql_parser.dialect = 'DuckDB';
1387+
1388+
statement ok
1389+
CREATE OR REPLACE TABLE addresses AS
1390+
SELECT '123 Quack Blvd' AS address, 'DuckTown' AS city, '11111' AS zip
1391+
UNION ALL
1392+
SELECT '111 Duck Duck Goose Ln', 'DuckTown', '11111'
1393+
UNION ALL
1394+
SELECT '111 Duck Duck Goose Ln', 'Duck Town', '11111'
1395+
UNION ALL
1396+
SELECT '111 Duck Duck Goose Ln', 'Duck Town', '11111-0001';
1397+
1398+
1399+
query TTT
1400+
SELECT * FROM addresses ORDER BY ALL;
1401+
----
1402+
111 Duck Duck Goose Ln Duck Town 11111
1403+
111 Duck Duck Goose Ln Duck Town 11111-0001
1404+
111 Duck Duck Goose Ln DuckTown 11111
1405+
123 Quack Blvd DuckTown 11111
1406+
1407+
query TTT
1408+
SELECT * FROM addresses ORDER BY ALL DESC;
1409+
----
1410+
123 Quack Blvd DuckTown 11111
1411+
111 Duck Duck Goose Ln DuckTown 11111
1412+
111 Duck Duck Goose Ln Duck Town 11111-0001
1413+
111 Duck Duck Goose Ln Duck Town 11111
1414+
1415+
query TT
1416+
SELECT address, zip FROM addresses ORDER BY ALL;
1417+
----
1418+
111 Duck Duck Goose Ln 11111
1419+
111 Duck Duck Goose Ln 11111
1420+
111 Duck Duck Goose Ln 11111-0001
1421+
123 Quack Blvd 11111

0 commit comments

Comments
 (0)