Skip to content

Commit 1ad989f

Browse files
LucaCappelletti94LucaCappelletti94
committed
Adressed PR reviews
1 parent 2d3c4f1 commit 1ad989f

File tree

10 files changed

+159
-239
lines changed

10 files changed

+159
-239
lines changed

‎src/ast/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ pub use self::query::{
9898
JsonTableNestedColumn, LateralView, LimitClause, LockClause, LockType, MatchRecognizePattern,
9999
MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset,
100100
OffsetRows, OpenJsonTableColumn, OrderBy, OrderByExpr, OrderByKind, OrderByOptions,
101-
PipeOperator, PivotValueSource, ProjectionSelect, Query, RenameSelectItem,
101+
OrderBySort, PipeOperator, PivotValueSource, ProjectionSelect, Query, RenameSelectItem,
102102
RepetitionQuantifier, ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select,
103103
SelectFlavor, SelectInto, SelectItem, SelectItemQualifiedWildcardKind, SelectModifiers,
104104
SetExpr, SetOperator, SetQuantifier, Setting, SymbolDefinition, Table, TableAlias,

‎src/ast/query.rs‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2883,9 +2883,7 @@ impl fmt::Display for OrderBy {
28832883
pub struct OrderByExpr {
28842884
/// The expression to order by.
28852885
pub expr: Expr,
2886-
/// Optional PostgreSQL `USING <operator>` clause.
2887-
pub using_operator: Option<ObjectName>,
2888-
/// Ordering options such as `ASC`/`DESC` and `NULLS` behavior.
2886+
/// Ordering options such as `ASC`/`DESC`/`USING <operator>` and `NULLS` behavior.
28892887
pub options: OrderByOptions,
28902888
/// Optional `WITH FILL` clause (ClickHouse extension) which specifies how to fill gaps.
28912889
pub with_fill: Option<WithFill>,
@@ -2895,7 +2893,6 @@ impl From<Ident> for OrderByExpr {
28952893
fn from(ident: Ident) -> Self {
28962894
OrderByExpr {
28972895
expr: Expr::Identifier(ident),
2898-
using_operator: None,
28992896
options: OrderByOptions::default(),
29002897
with_fill: None,
29012898
}
@@ -2905,13 +2902,6 @@ impl From<Ident> for OrderByExpr {
29052902
impl fmt::Display for OrderByExpr {
29062903
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
29072904
write!(f, "{}", self.expr)?;
2908-
if let Some(using_operator) = &self.using_operator {
2909-
if using_operator.0.len() > 1 {
2910-
write!(f, " USING OPERATOR({using_operator})")?;
2911-
} else {
2912-
write!(f, " USING {using_operator}")?;
2913-
}
2914-
}
29152905
write!(f, "{}", self.options)?;
29162906
if let Some(ref with_fill) = self.with_fill {
29172907
write!(f, " {with_fill}")?
@@ -2987,22 +2977,47 @@ impl fmt::Display for InterpolateExpr {
29872977
}
29882978
}
29892979

2990-
#[derive(Default, Debug, Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
2980+
/// The sort order for an `ORDER BY` expression.
2981+
///
2982+
/// See PostgreSQL `USING` operator:
2983+
/// <https://www.postgresql.org/docs/current/sql-select.html>
2984+
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
2985+
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
2986+
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
2987+
pub enum OrderBySort {
2988+
/// `ASC`
2989+
Asc,
2990+
/// `DESC`
2991+
Desc,
2992+
/// PostgreSQL `USING <operator>` ordering.
2993+
///
2994+
/// See <https://www.postgresql.org/docs/current/sql-select.html>
2995+
Using(ObjectName),
2996+
}
2997+
2998+
#[derive(Default, Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
29912999
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
29923000
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
2993-
/// Options for an `ORDER BY` expression (ASC/DESC and NULLS FIRST/LAST).
3001+
/// Options for an `ORDER BY` expression.
29943002
pub struct OrderByOptions {
2995-
/// Optional `ASC` (`Some(true)`) or `DESC` (`Some(false)`).
2996-
pub asc: Option<bool>,
3003+
/// Optional sort order: `ASC`, `DESC`, or `USING <operator>`.
3004+
pub sort: Option<OrderBySort>,
29973005
/// Optional `NULLS FIRST` (`Some(true)`) or `NULLS LAST` (`Some(false)`).
29983006
pub nulls_first: Option<bool>,
29993007
}
30003008

30013009
impl fmt::Display for OrderByOptions {
30023010
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
3003-
match self.asc {
3004-
Some(true) => write!(f, " ASC")?,
3005-
Some(false) => write!(f, " DESC")?,
3011+
match &self.sort {
3012+
Some(OrderBySort::Asc) => write!(f, " ASC")?,
3013+
Some(OrderBySort::Desc) => write!(f, " DESC")?,
3014+
Some(OrderBySort::Using(op)) => {
3015+
if op.0.len() > 1 {
3016+
write!(f, " USING OPERATOR({op})")?;
3017+
} else {
3018+
write!(f, " USING {op}")?;
3019+
}
3020+
}
30063021
None => (),
30073022
}
30083023
match self.nulls_first {

‎src/ast/spans.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2103,7 +2103,6 @@ impl Spanned for OrderByExpr {
21032103
fn span(&self) -> Span {
21042104
let OrderByExpr {
21052105
expr,
2106-
using_operator: _,
21072106
options: _,
21082107
with_fill,
21092108
} = self;

‎src/parser/mod.rs‎

Lines changed: 20 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,7 +1376,7 @@ impl<'a> Parser<'a> {
13761376
}
13771377
let alias = self.parse_optional_alias_inner(None, validator)?;
13781378
let order_by = OrderByOptions {
1379-
asc: self.parse_asc_desc(),
1379+
sort: self.parse_optional_order_by_sort(),
13801380
nulls_first: None,
13811381
};
13821382
Ok(ExprWithAliasAndOrderBy {
@@ -18285,6 +18285,15 @@ impl<'a> Parser<'a> {
1828518285
}
1828618286
}
1828718287

18288+
/// Parse ASC or DESC and map to [OrderBySort].
18289+
fn parse_optional_order_by_sort(&mut self) -> Option<OrderBySort> {
18290+
match self.parse_asc_desc() {
18291+
Some(true) => Some(OrderBySort::Asc),
18292+
Some(false) => Some(OrderBySort::Desc),
18293+
None => None,
18294+
}
18295+
}
18296+
1828818297
/// Parse an [OrderByExpr] expression.
1828918298
pub fn parse_order_by_expr(&mut self) -> Result<OrderByExpr, ParserError> {
1829018299
self.parse_order_by_expr_inner(false)
@@ -18321,28 +18330,14 @@ impl<'a> Parser<'a> {
1832118330
None
1832218331
};
1832318332

18324-
let using_operator = if !with_operator_class
18333+
let options = if !with_operator_class
1832518334
&& self.dialect.supports_order_by_using_operator()
1832618335
&& self.parse_keyword(Keyword::USING)
1832718336
{
18328-
Some(self.parse_order_by_using_operator()?)
18329-
} else {
18330-
None
18331-
};
18332-
18333-
let options = if using_operator.is_some() {
18334-
if self
18335-
.peek_one_of_keywords(&[Keyword::ASC, Keyword::DESC])
18336-
.is_some()
18337-
{
18338-
return parser_err!(
18339-
"ASC/DESC cannot be used together with USING in ORDER BY".to_string(),
18340-
self.peek_token_ref().span.start
18341-
);
18342-
}
18337+
let op = self.parse_order_by_using_operator()?;
1834318338
OrderByOptions {
18344-
asc: None,
18345-
nulls_first: self.parse_order_by_nulls_first_last(),
18339+
sort: Some(OrderBySort::Using(op)),
18340+
nulls_first: self.parse_null_ordering_modifier(),
1834618341
}
1834718342
} else {
1834818343
self.parse_order_by_options()?
@@ -18359,7 +18354,6 @@ impl<'a> Parser<'a> {
1835918354
Ok((
1836018355
OrderByExpr {
1836118356
expr,
18362-
using_operator,
1836318357
options,
1836418358
with_fill,
1836518359
},
@@ -18368,40 +18362,18 @@ impl<'a> Parser<'a> {
1836818362
}
1836918363

1837018364
fn parse_order_by_using_operator(&mut self) -> Result<ObjectName, ParserError> {
18371-
let dialect = self.dialect;
18372-
1837318365
if self.parse_keyword(Keyword::OPERATOR) {
1837418366
self.expect_token(&Token::LParen)?;
1837518367
let operator_name = self.parse_operator_name()?;
18376-
let Some(last_part) = operator_name.0.last() else {
18377-
return self.expected_ref("an operator name", self.peek_token_ref());
18378-
};
18379-
let operator = last_part.to_string();
18380-
if operator.is_empty()
18381-
|| !operator
18382-
.chars()
18383-
.all(|ch| dialect.is_custom_operator_part(ch))
18384-
{
18385-
return self.expected_ref("an operator name", self.peek_token_ref());
18386-
}
1838718368
self.expect_token(&Token::RParen)?;
1838818369
return Ok(operator_name);
1838918370
}
1839018371

1839118372
let token = self.next_token();
18392-
let operator = token.token.to_string();
18393-
if !operator.is_empty()
18394-
&& operator
18395-
.chars()
18396-
.all(|ch| dialect.is_custom_operator_part(ch))
18397-
{
18398-
Ok(ObjectName::from(vec![Ident::new(operator)]))
18399-
} else {
18400-
self.expected_ref("an ordering operator after USING", &token)
18401-
}
18373+
Ok(ObjectName::from(vec![Ident::new(token.token.to_string())]))
1840218374
}
1840318375

18404-
fn parse_order_by_nulls_first_last(&mut self) -> Option<bool> {
18376+
fn parse_null_ordering_modifier(&mut self) -> Option<bool> {
1840518377
if self.parse_keywords(&[Keyword::NULLS, Keyword::FIRST]) {
1840618378
Some(true)
1840718379
} else if self.parse_keywords(&[Keyword::NULLS, Keyword::LAST]) {
@@ -18412,10 +18384,10 @@ impl<'a> Parser<'a> {
1841218384
}
1841318385

1841418386
fn parse_order_by_options(&mut self) -> Result<OrderByOptions, ParserError> {
18415-
let asc = self.parse_asc_desc();
18416-
let nulls_first = self.parse_order_by_nulls_first_last();
18387+
let sort = self.parse_optional_order_by_sort();
18388+
let nulls_first = self.parse_null_ordering_modifier();
1841718389

18418-
Ok(OrderByOptions { asc, nulls_first })
18390+
Ok(OrderByOptions { sort, nulls_first })
1841918391
}
1842018392

1842118393
// Parse a WITH FILL clause (ClickHouse dialect)
@@ -20683,10 +20655,9 @@ mod tests {
2068320655
column: OrderByExpr {
2068420656
expr: Expr::Identifier(name.into()),
2068520657
options: OrderByOptions {
20686-
asc: None,
20658+
sort: None,
2068720659
nulls_first: None,
2068820660
},
20689-
using_operator: None,
2069020661
with_fill: None,
2069120662
},
2069220663
operator_class: None,

‎tests/sqlparser_bigquery.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2728,10 +2728,9 @@ fn test_export_data() {
27282728
kind: OrderByKind::Expressions(vec![OrderByExpr {
27292729
expr: Expr::Identifier(Ident::new("field1")),
27302730
options: OrderByOptions {
2731-
asc: None,
2731+
sort: None,
27322732
nulls_first: None,
27332733
},
2734-
using_operator: None,
27352734
with_fill: None,
27362735
},]),
27372736
interpolate: None,
@@ -2835,10 +2834,9 @@ fn test_export_data() {
28352834
kind: OrderByKind::Expressions(vec![OrderByExpr {
28362835
expr: Expr::Identifier(Ident::new("field1")),
28372836
options: OrderByOptions {
2838-
asc: None,
2837+
sort: None,
28392838
nulls_first: None,
28402839
},
2841-
using_operator: None,
28422840
with_fill: None,
28432841
},]),
28442842
interpolate: None,

‎tests/sqlparser_clickhouse.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,9 @@ fn parse_alter_table_add_projection() {
331331
kind: OrderByKind::Expressions(vec![OrderByExpr {
332332
expr: Identifier(Ident::new("b")),
333333
options: OrderByOptions {
334-
asc: None,
334+
sort: None,
335335
nulls_first: None,
336336
},
337-
using_operator: None,
338337
with_fill: None,
339338
}]),
340339
interpolate: None,
@@ -1160,10 +1159,9 @@ fn parse_select_order_by_with_fill_interpolate() {
11601159
OrderByExpr {
11611160
expr: Expr::Identifier(Ident::new("fname")),
11621161
options: OrderByOptions {
1163-
asc: Some(true),
1162+
sort: Some(OrderBySort::Asc),
11641163
nulls_first: Some(true),
11651164
},
1166-
using_operator: None,
11671165
with_fill: Some(WithFill {
11681166
from: Some(Expr::value(number("10"))),
11691167
to: Some(Expr::value(number("20"))),
@@ -1173,10 +1171,9 @@ fn parse_select_order_by_with_fill_interpolate() {
11731171
OrderByExpr {
11741172
expr: Expr::Identifier(Ident::new("lname")),
11751173
options: OrderByOptions {
1176-
asc: Some(false),
1174+
sort: Some(OrderBySort::Desc),
11771175
nulls_first: Some(false),
11781176
},
1179-
using_operator: None,
11801177
with_fill: Some(WithFill {
11811178
from: Some(Expr::value(number("30"))),
11821179
to: Some(Expr::value(number("40"))),

0 commit comments

Comments
 (0)