fmguerreiro commented on code in PR #2307:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2307#discussion_r3223390038


##########
src/parser/mod.rs:
##########
@@ -9926,6 +9952,71 @@ impl<'a> Parser<'a> {
         }
     }
 
+    fn parse_exclusion_element(&mut self) -> Result<ExclusionElement, 
ParserError> {
+        // `index_elem` grammar: { col | (expr) } [ opclass ] [ ASC | DESC ] [ 
NULLS FIRST | LAST ].
+        // Shared with `CREATE INDEX` columns.
+        let (
+            OrderByExpr {
+                expr,
+                options: order,
+                ..
+            },
+            operator_class,
+        ) = self.parse_order_by_expr_inner(true)?;
+
+        self.expect_keyword_is(Keyword::WITH)?;
+        let operator = self.parse_exclusion_operator()?;
+
+        Ok(ExclusionElement {
+            expr,
+            operator_class,
+            order,
+            operator,
+        })
+    }
+
+    /// Parse the operator that follows `WITH` in an `EXCLUDE` element.
+    ///
+    /// Accepts either a single operator token (e.g. `=`, `&&`, `<->`) or the
+    /// Postgres `OPERATOR(schema.op)` form for schema-qualified operators.
+    fn parse_exclusion_operator(&mut self) -> Result<ExclusionOperator, 
ParserError> {
+        if self.parse_keyword(Keyword::OPERATOR) {
+            return Ok(ExclusionOperator::PgCustom(
+                self.parse_pg_operator_ident_parts()?,
+            ));
+        }
+
+        let operator_token = self.next_token();
+        match &operator_token.token {
+            Token::EOF | Token::RParen | Token::Comma | Token::SemiColon => {

Review Comment:
   `RParen` is in the rejection list to catch the missing-operator case, not 
because it is an optional valid operator. After `WITH`, the parser consumes one 
token as the operator. If that token is `)`, `,`, `;`, or `EOF`, the user wrote 
something like `col WITH)` or `col WITH,` — a structural delimiter where an 
operator should appear. Treating those tokens as operator strings would produce 
a silently wrong AST, so the function errors immediately with "expected 
exclusion operator". The `)` that closes the element list will be consumed by 
the caller (`parse_exclude_constraint`) via `expect_token(&Token::RParen)` 
after `parse_comma_separated` returns.



##########
tests/sqlparser_postgres.rs:
##########
@@ -9193,3 +9243,310 @@ fn parse_lock_table() {
         }
     }
 }
+
+#[test]
+fn parse_exclude_constraint_with_where() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) WHERE 
(col > 0))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert!(c.where_clause.is_some());
+                    match c.where_clause.as_ref().unwrap().as_ref() {
+                        Expr::BinaryOp { left, op, right } => {
+                            assert_eq!(**left, 
Expr::Identifier(Ident::new("col")));
+                            assert_eq!(*op, BinaryOperator::Gt);
+                            assert_eq!(**right, 
Expr::Value(number("0").with_empty_span()));
+                        }
+                        other => panic!("Expected BinaryOp, got {other:?}"),
+                    }
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_with_include() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) 
INCLUDE (col))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert_eq!(c.elements.len(), 1);
+                    assert_eq!(c.include, vec![Ident::new("col")]);
+                    assert!(c.where_clause.is_none());
+                    assert!(c.characteristics.is_none());
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_no_using() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE (col WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert!(c.index_method.is_none());
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_deferrable() {
+    let sql =
+        "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) DEFERRABLE 
INITIALLY DEFERRED)";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    let characteristics = c.characteristics.as_ref().unwrap();
+                    assert_eq!(characteristics.deferrable, Some(true));
+                    assert_eq!(characteristics.initially, 
Some(DeferrableInitial::Deferred));
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_in_alter_table() {
+    let sql = "ALTER TABLE t ADD CONSTRAINT no_overlap EXCLUDE USING gist 
(room WITH =)";
+    match pg().verified_stmt(sql) {
+        Statement::AlterTable(alter_table) => match &alter_table.operations[0] 
{
+            AlterTableOperation::AddConstraint {
+                constraint: TableConstraint::Exclusion(c),
+                ..
+            } => {
+                assert_eq!(c.name, Some(Ident::new("no_overlap")));
+                assert_eq!(c.elements[0].operator.to_string(), "=");
+            }
+            other => panic!("Expected AddConstraint(Exclusion), got 
{other:?}"),
+        },
+        _ => panic!("Expected AlterTable"),
+    }
+}
+
+#[test]
+fn roundtrip_exclude_constraint() {
+    let sql = "CREATE TABLE t (CONSTRAINT no_overlap EXCLUDE USING gist (room 
WITH =, during WITH &&) INCLUDE (id) WHERE (active = true))";

Review Comment:
   Done in 60b5fd2: all tests were merged into a single 
`parse_exclude_constraint` function in `tests/sqlparser_common.rs`. The one 
AST-asserting case is kept; everything else is covered by `verified_stmt` 
round-trips.



##########
tests/sqlparser_postgres.rs:
##########
@@ -9193,3 +9243,310 @@ fn parse_lock_table() {
         }
     }
 }
+
+#[test]
+fn parse_exclude_constraint_with_where() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) WHERE 
(col > 0))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert!(c.where_clause.is_some());
+                    match c.where_clause.as_ref().unwrap().as_ref() {
+                        Expr::BinaryOp { left, op, right } => {
+                            assert_eq!(**left, 
Expr::Identifier(Ident::new("col")));
+                            assert_eq!(*op, BinaryOperator::Gt);
+                            assert_eq!(**right, 
Expr::Value(number("0").with_empty_span()));
+                        }
+                        other => panic!("Expected BinaryOp, got {other:?}"),
+                    }
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_with_include() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) 
INCLUDE (col))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert_eq!(c.elements.len(), 1);
+                    assert_eq!(c.include, vec![Ident::new("col")]);
+                    assert!(c.where_clause.is_none());
+                    assert!(c.characteristics.is_none());
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_no_using() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE (col WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    assert!(c.index_method.is_none());
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_deferrable() {
+    let sql =
+        "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) DEFERRABLE 
INITIALLY DEFERRED)";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => {
+            assert_eq!(1, create_table.constraints.len());
+            match &create_table.constraints[0] {
+                TableConstraint::Exclusion(c) => {
+                    let characteristics = c.characteristics.as_ref().unwrap();
+                    assert_eq!(characteristics.deferrable, Some(true));
+                    assert_eq!(characteristics.initially, 
Some(DeferrableInitial::Deferred));
+                }
+                other => panic!("Expected Exclusion, got {other:?}"),
+            }
+        }
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_in_alter_table() {
+    let sql = "ALTER TABLE t ADD CONSTRAINT no_overlap EXCLUDE USING gist 
(room WITH =)";
+    match pg().verified_stmt(sql) {
+        Statement::AlterTable(alter_table) => match &alter_table.operations[0] 
{
+            AlterTableOperation::AddConstraint {
+                constraint: TableConstraint::Exclusion(c),
+                ..
+            } => {
+                assert_eq!(c.name, Some(Ident::new("no_overlap")));
+                assert_eq!(c.elements[0].operator.to_string(), "=");
+            }
+            other => panic!("Expected AddConstraint(Exclusion), got 
{other:?}"),
+        },
+        _ => panic!("Expected AlterTable"),
+    }
+}
+
+#[test]
+fn roundtrip_exclude_constraint() {
+    let sql = "CREATE TABLE t (CONSTRAINT no_overlap EXCLUDE USING gist (room 
WITH =, during WITH &&) INCLUDE (id) WHERE (active = true))";
+    pg().verified_stmt(sql);
+}
+
+#[test]
+fn parse_exclude_constraint_not_deferrable_initially_immediate() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =) NOT 
DEFERRABLE INITIALLY IMMEDIATE)";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => {
+                let characteristics = c.characteristics.as_ref().unwrap();
+                assert_eq!(characteristics.deferrable, Some(false));
+                assert_eq!(
+                    characteristics.initially,
+                    Some(DeferrableInitial::Immediate)
+                );
+            }
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_collate() {
+    // `COLLATE` is consumed by the element expression parser; verify that
+    // a collated column round-trips inside an EXCLUDE element.
+    pg().verified_stmt(
+        "CREATE TABLE t (name TEXT, EXCLUDE USING btree (name COLLATE \"C\" 
WITH =))",
+    );
+}
+
+#[test]
+fn parse_exclude_constraint_operator_class() {
+    let sql = "CREATE TABLE t (col TEXT, EXCLUDE USING gist (col 
text_pattern_ops WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => {
+                assert_eq!(c.elements.len(), 1);
+                assert_eq!(c.elements[0].expr, 
Expr::Identifier(Ident::new("col")));
+                assert_eq!(
+                    c.elements[0].operator_class,
+                    
Some(ObjectName::from(vec![Ident::new("text_pattern_ops")]))
+                );
+                assert_eq!(c.elements[0].operator.to_string(), "=");
+            }
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_asc_nulls_last() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING btree (col ASC NULLS 
LAST WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => {
+                assert_eq!(c.elements[0].order.asc, Some(true));
+                assert_eq!(c.elements[0].order.nulls_first, Some(false));
+            }
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_desc_nulls_first() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING btree (col DESC NULLS 
FIRST WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => {
+                assert_eq!(c.elements[0].order.asc, Some(false));
+                assert_eq!(c.elements[0].order.nulls_first, Some(true));
+            }
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_function_expression() {
+    let sql =
+        "CREATE TABLE t (name TEXT, EXCLUDE USING gist ((lower(name)) 
text_pattern_ops WITH =))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => {
+                assert_eq!(c.elements.len(), 1);
+                match &c.elements[0].expr {
+                    Expr::Nested(inner) => match inner.as_ref() {
+                        Expr::Function(func) => {
+                            assert_eq!(func.name.to_string(), "lower");
+                        }
+                        other => panic!("Expected Function inside Nested, got 
{other:?}"),
+                    },
+                    other => panic!("Expected Nested expr, got {other:?}"),
+                }
+                assert_eq!(
+                    c.elements[0].operator_class,
+                    
Some(ObjectName::from(vec![Ident::new("text_pattern_ops")]))
+                );
+                assert_eq!(c.elements[0].operator.to_string(), "=");
+            }
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn parse_exclude_constraint_pg_custom_operator() {
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH 
OPERATOR(pg_catalog.=)))";
+    match pg().verified_stmt(sql) {
+        Statement::CreateTable(create_table) => match 
&create_table.constraints[0] {
+            TableConstraint::Exclusion(c) => match &c.elements[0].operator {
+                ExclusionOperator::PgCustom(parts) => {
+                    assert_eq!(parts, &vec!["pg_catalog".to_string(), 
"=".to_string()]);
+                }
+                other => panic!("Expected PgCustom operator, got {other:?}"),
+            },
+            other => panic!("Expected Exclusion, got {other:?}"),
+        },
+        _ => panic!("Expected CreateTable"),
+    }
+}
+
+#[test]
+fn exclude_missing_with_keyword_errors() {
+    let sql = "CREATE TABLE t (CONSTRAINT c EXCLUDE USING gist (col))";
+    let err = pg().parse_sql_statements(sql).unwrap_err();
+    assert!(
+        err.to_string().contains("Expected: WITH"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn exclude_empty_element_list_errors() {
+    let sql = "CREATE TABLE t (CONSTRAINT c EXCLUDE USING gist ())";
+    let err = pg().parse_sql_statements(sql).unwrap_err();
+    assert!(
+        err.to_string().contains("Expected: an expression"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn exclude_missing_operator_errors() {
+    let sql = "CREATE TABLE t (CONSTRAINT c EXCLUDE USING gist (col WITH))";
+    let err = pg().parse_sql_statements(sql).unwrap_err();
+    assert!(
+        err.to_string().contains("exclusion operator"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn parse_exclude_constraint_operator_with_ordering() {
+    pg().verified_stmt(
+        "CREATE TABLE t (col INT, CONSTRAINT c EXCLUDE USING gist (col ASC 
WITH OPERATOR(pg_catalog.=)))",
+    );
+}
+
+#[test]
+fn exclude_rejected_in_non_postgres_dialects() {
+    // `GenericDialect` is intentionally excluded — it opts in to the
+    // Postgres EXCLUDE syntax alongside `PostgreSqlDialect`.
+    let sql = "CREATE TABLE t (col INT, EXCLUDE USING gist (col WITH =))";
+    for dialect in
+        all_dialects_except(|d| d.is::<PostgreSqlDialect>() || 
d.is::<GenericDialect>()).dialects
+    {
+        let parser = TestedDialects::new(vec![dialect]);
+        assert!(
+            parser.parse_sql_statements(sql).is_err(),
+            "dialect unexpectedly accepted EXCLUDE: {sql}"
+        );
+    }
+}
+
+#[test]
+fn exclude_as_column_name_parses_in_mysql_and_sqlite() {
+    // `exclude` must remain usable as an identifier where it is not a
+    // reserved keyword; PG reserves it as a constraint keyword.

Review Comment:
   Moved to `tests/sqlparser_common.rs` in 60b5fd2, gated on 
`all_dialects_where(|d| d.supports_exclude_constraint())`.



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