iffyio commented on code in PR #2246:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2246#discussion_r2965601434
##########
src/parser/mod.rs:
##########
@@ -18169,23 +18194,61 @@ impl<'a> Parser<'a> {
Ok((
OrderByExpr {
expr,
+ using_operator,
options,
with_fill,
},
operator_class,
))
}
- fn parse_order_by_options(&mut self) -> Result<OrderByOptions,
ParserError> {
- let asc = self.parse_asc_desc();
+ fn parse_order_by_using_operator(&mut self) -> Result<ObjectName,
ParserError> {
+ let dialect = self.dialect;
- let nulls_first = if self.parse_keywords(&[Keyword::NULLS,
Keyword::FIRST]) {
+ if self.parse_keyword(Keyword::OPERATOR) {
+ self.expect_token(&Token::LParen)?;
+ let operator_name = self.parse_operator_name()?;
+ let Some(last_part) = operator_name.0.last() else {
+ return self.expected_ref("an operator name",
self.peek_token_ref());
+ };
+ let operator = last_part.to_string();
+ if operator.is_empty()
+ || !operator
+ .chars()
+ .all(|ch| dialect.is_custom_operator_part(ch))
+ {
+ return self.expected_ref("an operator name",
self.peek_token_ref());
+ }
+ self.expect_token(&Token::RParen)?;
+ return Ok(operator_name);
+ }
+
+ let token = self.next_token();
+ let operator = token.token.to_string();
+ if !operator.is_empty()
+ && operator
+ .chars()
+ .all(|ch| dialect.is_custom_operator_part(ch))
+ {
+ Ok(ObjectName::from(vec![Ident::new(operator)]))
+ } else {
+ self.expected_ref("an ordering operator after USING", &token)
+ }
+ }
+
+ fn parse_order_by_nulls_first_last(&mut self) -> Option<bool> {
Review Comment:
```suggestion
fn parse_null_ordering_modifier(&mut self) -> Option<bool> {
```
##########
src/parser/mod.rs:
##########
@@ -18169,23 +18194,61 @@ impl<'a> Parser<'a> {
Ok((
OrderByExpr {
expr,
+ using_operator,
options,
with_fill,
},
operator_class,
))
}
- fn parse_order_by_options(&mut self) -> Result<OrderByOptions,
ParserError> {
- let asc = self.parse_asc_desc();
+ fn parse_order_by_using_operator(&mut self) -> Result<ObjectName,
ParserError> {
+ let dialect = self.dialect;
- let nulls_first = if self.parse_keywords(&[Keyword::NULLS,
Keyword::FIRST]) {
+ if self.parse_keyword(Keyword::OPERATOR) {
+ self.expect_token(&Token::LParen)?;
+ let operator_name = self.parse_operator_name()?;
+ let Some(last_part) = operator_name.0.last() else {
+ return self.expected_ref("an operator name",
self.peek_token_ref());
+ };
+ let operator = last_part.to_string();
+ if operator.is_empty()
+ || !operator
+ .chars()
+ .all(|ch| dialect.is_custom_operator_part(ch))
+ {
+ return self.expected_ref("an operator name",
self.peek_token_ref());
+ }
Review Comment:
the `to_string()` cloning doesn't seem to be required here? But in general
this logic iiuc is validating some property of the allowed naming? if so I
think it would make sense to skip the validation here in the parser (same for
the similar logic below, so that if the code is to be kept then we likely want
to pull it out to a reusable function)
##########
src/parser/mod.rs:
##########
@@ -18156,7 +18156,32 @@ impl<'a> Parser<'a> {
None
};
- let options = self.parse_order_by_options()?;
+ let using_operator = if !with_operator_class
+ && self.dialect.supports_order_by_using_operator()
+ && self.parse_keyword(Keyword::USING)
+ {
+ Some(self.parse_order_by_using_operator()?)
+ } else {
+ None
+ };
+
+ let options = if using_operator.is_some() {
+ if self
+ .peek_one_of_keywords(&[Keyword::ASC, Keyword::DESC])
+ .is_some()
+ {
+ return parser_err!(
+ "ASC/DESC cannot be used together with USING in ORDER
BY".to_string(),
+ self.peek_token_ref().span.start
+ );
+ }
+ OrderByOptions {
+ asc: None,
+ nulls_first: self.parse_order_by_nulls_first_last(),
Review Comment:
I think we can skip this validation
##########
src/ast/query.rs:
##########
@@ -2860,6 +2860,8 @@ impl fmt::Display for OrderBy {
pub struct OrderByExpr {
/// The expression to order by.
pub expr: Expr,
+ /// Optional PostgreSQL `USING <operator>` clause.
Review Comment:
fyi: re the comment would be nice to include the link
##########
tests/sqlparser_postgres.rs:
##########
@@ -5727,6 +5729,101 @@ fn parse_array_agg() {
pg().verified_stmt(sql4);
}
+#[test]
+fn parse_pg_aggregate_order_by_using_operator() {
+ let sql = "SELECT aggfns(DISTINCT a, a, c ORDER BY c USING ~<~, a) FROM t";
Review Comment:
can we move the tests to common and use the `all_dialects_where` helper
method for the test cases?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
