fmguerreiro commented on code in PR #2307:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2307#discussion_r3140755528
##########
src/parser/mod.rs:
##########
@@ -9915,9 +9895,55 @@ impl<'a> Parser<'a> {
.into(),
))
}
+ Token::Word(w)
+ if w.keyword == Keyword::EXCLUDE
+ && dialect_of!(self is PostgreSqlDialect | GenericDialect)
=>
Review Comment:
Added `Dialect::supports_exclude_constraint()` (default `false`, enabled for
`PostgreSqlDialect` and `GenericDialect`) and replaced the macro guard with it
in 60b5fd2.
##########
src/parser/mod.rs:
##########
@@ -9915,9 +9895,55 @@ impl<'a> Parser<'a> {
.into(),
))
}
+ Token::Word(w)
+ if w.keyword == Keyword::EXCLUDE
+ && dialect_of!(self is PostgreSqlDialect | GenericDialect)
=>
+ {
+ let index_method = if self.parse_keyword(Keyword::USING) {
Review Comment:
Extracted into `parse_exclude_constraint` in 60b5fd2.
##########
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 ].
Review Comment:
Done in 60b5fd2.
##########
src/parser/mod.rs:
##########
@@ -9926,6 +9952,71 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_exclusion_element(&mut self) -> Result<ExclusionElement,
ParserError> {
Review Comment:
Renamed to `parse_exclude_constraint_element` in 60b5fd2 (the return type is
now `ExcludeConstraintElement` to match the AST rename).
##########
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.
Review Comment:
Done in 60b5fd2.
##########
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.
Review Comment:
Done in 60b5fd2.
##########
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,
Review Comment:
Switched to `parse_create_index_expr` (the public wrapper around
`parse_order_by_expr_inner`) and now store the resulting `IndexColumn` directly
on `ExcludeConstraintElement` instead of destructuring an `OrderByExpr` — see
60b5fd2.
##########
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> {
Review Comment:
Renamed to `parse_exclude_constraint_operator` (returning
`ExcludeConstraintOperator`) in 60b5fd2.
##########
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:
Not optional — it was guarding against `WITH` being followed directly by a
structural delimiter (`,`, `)`, `;`, EOF), i.e. a missing operator. Rewrote it
as an explicit `if matches!` with a comment spelling out the case in 60b5fd2.
##########
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:
Collapsed the per-form tests into a single `parse_exclude_constraint` driven
by `verified_stmt` (with one explicit AST-asserting case to lock the structure)
in 60b5fd2.
--
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]
