iffyio commented on code in PR #2246:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2246#discussion_r2883871866


##########
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";
+    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"))
+    );
+
+    #[derive(Debug)]
+    struct OrderByUsingDisabledDialect;
+
+    impl Dialect for OrderByUsingDisabledDialect {
+        fn is_identifier_start(&self, ch: char) -> bool {
+            PostgreSqlDialect {}.is_identifier_start(ch)
+        }
+
+        fn is_identifier_part(&self, ch: char) -> bool {
+            PostgreSqlDialect {}.is_identifier_part(ch)
+        }
+
+        fn supports_order_by_using_operator(&self) -> bool {
+            false
+        }
+    }
+
+    let without_order_by_using = 
TestedDialects::new(vec![Box::new(OrderByUsingDisabledDialect)]);
+    assert!(without_order_by_using
+        .parse_sql_statements("SELECT a FROM t ORDER BY a USING <;")

Review Comment:
   I think we can have the tests in common.rs instead to cover dialects via the 
`all_dialects_where(|d| d.support*)` pattern. Since the impl is guarded by the 
dialect method vs explicit to postgres



##########
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:
   can we include reference doc to maybe this?
   https://www.postgresql.org/docs/current/sql-select.html
   
   so folks know where to find this feature easily



##########
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:
   would it be possible to use something like this representation instead?
   
   ```rust
   pub enum OrderByExpr {
       Asc,
       Desc,
       Using(...),
       UsingOperator(...),
   }
   
   pub struct OrderByOptions {
       pub expr: Option<OrderByExpr>,
       pub nulls_first: Option<bool>,
   }
   ```



-- 
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]

Reply via email to