iffyio commented on code in PR #2246:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2246#discussion_r2872045220
##########
src/parser/mod.rs:
##########
@@ -18083,7 +18072,32 @@ impl<'a> Parser<'a> {
None
};
- let options = self.parse_order_by_options()?;
+ let using_operator = if !with_operator_class
+ && dialect_of!(self is PostgreSqlDialect)
Review Comment:
if we need to dialect guard this, I think we can introduce a dialect method
instead of the macro
##########
src/parser/mod.rs:
##########
@@ -18096,23 +18110,76 @@ 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> {
+ 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 !Self::is_valid_order_by_using_operator_symbol(&operator) {
+ return self.expected_ref("an operator name",
self.peek_token_ref());
+ }
+ self.expect_token(&Token::RParen)?;
+ return Ok(operator_name);
+ }
- let nulls_first = if self.parse_keywords(&[Keyword::NULLS,
Keyword::FIRST]) {
+ let token = self.next_token();
+ let operator = token.token.to_string();
+ if Self::is_valid_order_by_using_operator_symbol(&operator) {
+ Ok(ObjectName::from(vec![Ident::new(operator)]))
+ } else {
+ self.expected_ref("an ordering operator after USING", &token)
+ }
+ }
+
+ fn is_valid_order_by_using_operator_symbol(symbol: &str) -> bool {
Review Comment:
is this code necessary?
##########
src/parser/mod.rs:
##########
@@ -4990,10 +4989,10 @@ impl<'a> Parser<'a> {
loop {
match &self.peek_nth_token_ref(0).token {
Token::EOF => break,
- Token::Word(w) => {
- if w.quote_style.is_none() &&
terminal_keywords.contains(&w.keyword) {
- break;
- }
+ Token::Word(w)
+ if w.quote_style.is_none() &&
terminal_keywords.contains(&w.keyword) =>
+ {
+ break;
Review Comment:
are these diffs required for this PR?
##########
tests/sqlparser_postgres.rs:
##########
@@ -5685,6 +5687,85 @@ 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";
+ let select = pg().verified_only_select(sql);
+ let SelectItem::UnnamedExpr(Expr::Function(Function {
+ args: FunctionArguments::List(FunctionArgumentList { clauses, .. }),
+ ..
+ })) = &select.projection[0]
+ else {
+ unreachable!("expected aggregate function in projection");
+ };
+
+ let Some(FunctionArgumentClause::OrderBy(order_by_exprs)) = clauses
+ .iter()
+ .find(|clause| matches!(clause, FunctionArgumentClause::OrderBy(_)))
+ else {
+ unreachable!("expected ORDER BY clause in aggregate function argument
list");
+ };
+
+ assert_eq!(
+ order_by_exprs[0].using_operator,
+ Some(ObjectName::from(vec!["~<~".into()]))
+ );
+ assert_eq!(order_by_exprs[1].using_operator, None);
+}
+
+#[test]
+fn parse_pg_order_by_using_operator_syntax() {
+ pg().one_statement_parses_to(
+ "SELECT a FROM t ORDER BY a USING OPERATOR(<)",
+ "SELECT a FROM t ORDER BY a USING <",
+ );
+
+ let query =
+ pg().verified_query("SELECT a FROM t ORDER BY a USING
OPERATOR(pg_catalog.<) NULLS LAST");
+ let order_by = query.order_by.expect("expected ORDER BY clause");
+ let OrderByKind::Expressions(exprs) = order_by.kind else {
+ unreachable!("expected ORDER BY expressions");
+ };
+
+ assert_eq!(
+ exprs[0].using_operator,
+ Some(ObjectName::from(vec![
+ Ident::new("pg_catalog"),
+ Ident::new("<"),
+ ]))
+ );
+ assert_eq!(exprs[0].options.asc, None);
+ assert_eq!(exprs[0].options.nulls_first, Some(false));
+}
+
+#[test]
+fn parse_pg_order_by_using_operator_invalid_cases() {
+ let err = pg()
+ .parse_sql_statements("SELECT a FROM t ORDER BY a USING ;")
+ .unwrap_err();
+ assert!(
+ matches!(err, ParserError::ParserError(msg) if msg.contains("an
ordering operator after USING"))
+ );
+
+ let err = pg()
+ .parse_sql_statements("SELECT a FROM t ORDER BY a USING OPERATOR();")
+ .unwrap_err();
+ assert!(matches!(err, ParserError::ParserError(msg) if msg.contains("an
operator name")));
+
+ let err = pg()
+ .parse_sql_statements("SELECT a FROM t ORDER BY a USING < DESC;")
+ .unwrap_err();
+ assert!(
+ matches!(err, ParserError::ParserError(msg) if msg.contains("ASC/DESC
cannot be used together with USING in ORDER BY"))
+ );
+
+ // `USING` in ORDER BY is PostgreSQL-specific and should not parse in
GenericDialect.
+ let generic = TestedDialects::new(vec![Box::new(GenericDialect {})]);
Review Comment:
note, per the dialect comment, we'd want to avoid hardcoding dialects in
tests
--
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]
