fix(exclude): address upstream review feedback on PR #2307

fmguerreiro · fmguerreiro · commit 60b5fd253a52 · 2026-04-25T08:17:29.000+09:00
- Rename `ExclusionConstraint`/`ExclusionElement`/`ExclusionOperator`
  (and `TableConstraint::Exclusion`) to `ExcludeConstraint`/
  `ExcludeConstraintElement`/`ExcludeConstraintOperator`/
  `TableConstraint::Exclude` to match the SQL keyword.
- Tighten doc comments on the new types: add the canonical
  `[PostgreSQL](...)` doc link to each and drop redundant grammar
  sketches where the variant doc already carries it.
- Introduce `Dialect::supports_exclude_constraint()` (default `false`,
  enabled for `PostgreSqlDialect` and `GenericDialect`) and replace
  the inline `dialect_of!` macro guard with the new method.
- Extract the `EXCLUDE` parsing into `parse_exclude_constraint`,
  `parse_exclude_constraint_element`, and
  `parse_exclude_constraint_operator` instead of inlining the body of
  the constraint match arm.
- Switch `ExcludeConstraintElement` to embed `IndexColumn` directly,
  parsed via `parse_create_index_expr`, instead of destructuring an
  `OrderByExpr`.
- Replace the structural-delimiter `match` in
  `parse_exclude_constraint_operator` with an explicit `if matches!`
  guard plus a comment explaining the missing-operator case.
- Move the `EXCLUDE` constraint tests out of `sqlparser_postgres.rs`
  into a single `parse_exclude_constraint` test in
  `sqlparser_common.rs` driven by
  `all_dialects_where(|d| d.supports_exclude_constraint())`, leaning
  on `verified_stmt` round-trips for breadth.
diff --git a/src/ast/mod.rs b/src/ast/mod.rs
@@ -138,8 +138,8 @@ mod dml;
 pub mod helpers;
 pub mod table_constraints;
 pub use table_constraints::{
-    CheckConstraint, ConstraintUsingIndex, ExclusionConstraint, ExclusionElement,
-    ExclusionOperator, ForeignKeyConstraint, FullTextOrSpatialConstraint, IndexConstraint,
+    CheckConstraint, ConstraintUsingIndex, ExcludeConstraint, ExcludeConstraintElement,
+    ExcludeConstraintOperator, ForeignKeyConstraint, FullTextOrSpatialConstraint, IndexConstraint,
     PrimaryKeyConstraint, TableConstraint, UniqueConstraint,
 };
 mod operator;
diff --git a/src/ast/spans.rs b/src/ast/spans.rs
@@ -650,7 +650,7 @@ impl Spanned for TableConstraint {
             TableConstraint::FulltextOrSpatial(constraint) => constraint.span(),
             TableConstraint::PrimaryKeyUsingIndex(constraint)
             | TableConstraint::UniqueUsingIndex(constraint) => constraint.span(),
-            TableConstraint::Exclusion(constraint) => constraint.span(),
+            TableConstraint::Exclude(constraint) => constraint.span(),
         }
     }
 }
diff --git a/src/ast/table_constraints.rs b/src/ast/table_constraints.rs
@@ -20,7 +20,7 @@
 use crate::ast::{
     display_comma_separated, display_separated, ConstraintCharacteristics,
     ConstraintReferenceMatchKind, Expr, Ident, IndexColumn, IndexOption, IndexType,
-    KeyOrIndexDisplay, NullsDistinctOption, ObjectName, OrderByOptions, ReferentialAction,
+    KeyOrIndexDisplay, NullsDistinctOption, ObjectName, ReferentialAction,
 };
 use crate::tokenizer::Span;
 use core::fmt;
@@ -117,12 +117,12 @@ pub enum TableConstraint {
     ///
     /// [1]: https://www.postgresql.org/docs/current/sql-altertable.html
     UniqueUsingIndex(ConstraintUsingIndex),
-    /// PostgreSQL `EXCLUDE` constraint.
+    /// `EXCLUDE` constraint.
     ///
     /// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]`
     ///
-    /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE>
-    Exclusion(ExclusionConstraint),
+    /// [PostgreSQL](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE)
+    Exclude(ExcludeConstraint),
 }
 
 impl From<UniqueConstraint> for TableConstraint {
@@ -161,9 +161,9 @@ impl From<FullTextOrSpatialConstraint> for TableConstraint {
     }
 }
 
-impl From<ExclusionConstraint> for TableConstraint {
-    fn from(constraint: ExclusionConstraint) -> Self {
-        TableConstraint::Exclusion(constraint)
+impl From<ExcludeConstraint> for TableConstraint {
+    fn from(constraint: ExcludeConstraint) -> Self {
+        TableConstraint::Exclude(constraint)
     }
 }
 
@@ -178,7 +178,7 @@ impl fmt::Display for TableConstraint {
             TableConstraint::FulltextOrSpatial(constraint) => constraint.fmt(f),
             TableConstraint::PrimaryKeyUsingIndex(c) => c.fmt_with_keyword(f, "PRIMARY KEY"),
             TableConstraint::UniqueUsingIndex(c) => c.fmt_with_keyword(f, "UNIQUE"),
-            TableConstraint::Exclusion(constraint) => constraint.fmt(f),
+            TableConstraint::Exclude(constraint) => constraint.fmt(f),
         }
     }
 }
@@ -617,22 +617,24 @@ impl crate::ast::Spanned for ConstraintUsingIndex {
     }
 }
 
-/// The operator that follows `WITH` in an `EXCLUDE` element.
+/// The operator that follows `WITH` in an `EXCLUDE` constraint element.
+///
+/// [PostgreSQL](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE)
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
-pub enum ExclusionOperator {
+pub enum ExcludeConstraintOperator {
     /// A single operator token, e.g. `=`, `&&`, `<->`.
     Token(String),
     /// Postgres schema-qualified form: `OPERATOR(schema.op)`.
     PgCustom(Vec<String>),
 }
 
-impl fmt::Display for ExclusionOperator {
+impl fmt::Display for ExcludeConstraintOperator {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            ExclusionOperator::Token(token) => f.write_str(token),
-            ExclusionOperator::PgCustom(parts) => {
+            ExcludeConstraintOperator::Token(token) => f.write_str(token),
+            ExcludeConstraintOperator::PgCustom(parts) => {
                 write!(f, "OPERATOR({})", display_separated(parts, "."))
             }
         }
@@ -641,58 +643,46 @@ impl fmt::Display for ExclusionOperator {
 
 /// One element in an `EXCLUDE` constraint's element list.
 ///
-/// `{ column_name | ( expression ) } [ opclass ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] WITH <operator>`
-///
-/// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE>
+/// [PostgreSQL](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE)
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
-pub struct ExclusionElement {
-    /// The index expression or column name.
-    pub expr: Expr,
-    /// Optional operator class (e.g. `gist_geometry_ops_nd`).
-    pub operator_class: Option<ObjectName>,
-    /// Ordering options (ASC/DESC, NULLS FIRST/LAST).
-    pub order: OrderByOptions,
+pub struct ExcludeConstraintElement {
+    /// The index column (`{ column_name | ( expression ) } [ opclass ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ]`).
+    pub column: IndexColumn,
     /// The exclusion operator.
-    pub operator: ExclusionOperator,
+    pub operator: ExcludeConstraintOperator,
 }
 
-impl fmt::Display for ExclusionElement {
+impl fmt::Display for ExcludeConstraintElement {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.expr)?;
-        if let Some(opclass) = &self.operator_class {
-            write!(f, " {opclass}")?;
-        }
-        write!(f, "{} WITH {}", self.order, self.operator)
+        write!(f, "{} WITH {}", self.column, self.operator)
     }
 }
 
-impl crate::ast::Spanned for ExclusionElement {
+impl crate::ast::Spanned for ExcludeConstraintElement {
     fn span(&self) -> Span {
-        let mut span = self.expr.span();
-        if let Some(opclass) = &self.operator_class {
+        let mut span = self.column.column.expr.span();
+        if let Some(opclass) = &self.column.operator_class {
             span = span.union(&opclass.span());
         }
         span
     }
 }
 
-/// A PostgreSQL `EXCLUDE` constraint.
-///
-/// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]`
+/// An `EXCLUDE` constraint.
 ///
-/// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE>
+/// [PostgreSql](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE)
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
-pub struct ExclusionConstraint {
+pub struct ExcludeConstraint {
     /// Optional constraint name.
     pub name: Option<Ident>,
     /// Optional index method (e.g. `gist`, `spgist`).
     pub index_method: Option<Ident>,
     /// The list of index expressions with their exclusion operators.
-    pub elements: Vec<ExclusionElement>,
+    pub elements: Vec<ExcludeConstraintElement>,
     /// Optional list of additional columns to include in the index.
     pub include: Vec<Ident>,
     /// Optional `WHERE` predicate to restrict the constraint to a subset of rows.
@@ -701,7 +691,7 @@ pub struct ExclusionConstraint {
     pub characteristics: Option<ConstraintCharacteristics>,
 }
 
-impl fmt::Display for ExclusionConstraint {
+impl fmt::Display for ExcludeConstraint {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use crate::ast::ddl::display_constraint_name;
         write!(f, "{}EXCLUDE", display_constraint_name(&self.name))?;
@@ -722,7 +712,7 @@ impl fmt::Display for ExclusionConstraint {
     }
 }
 
-impl crate::ast::Spanned for ExclusionConstraint {
+impl crate::ast::Spanned for ExcludeConstraint {
     fn span(&self) -> Span {
         Span::union_iter(
             self.name
diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs
@@ -129,6 +129,10 @@ impl Dialect for GenericDialect {
         true
     }
 
+    fn supports_exclude_constraint(&self) -> bool {
+        true
+    }
+
     fn supports_limit_comma(&self) -> bool {
         true
     }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
@@ -1167,6 +1167,13 @@ pub trait Dialect: Debug + Any {
         false
     }
 
+    /// Returns true if the dialect supports `EXCLUDE` table constraints, e.g.
+    /// `EXCLUDE USING gist (c WITH &&)` in `CREATE TABLE`/`ALTER TABLE`.
+    /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE>.
+    fn supports_exclude_constraint(&self) -> bool {
+        false
+    }
+
     /// Returns true if the dialect supports the `LOAD DATA` statement
     fn supports_load_data(&self) -> bool {
         false
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
@@ -208,6 +208,11 @@ impl Dialect for PostgreSqlDialect {
         true
     }
 
+    /// see <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE>
+    fn supports_exclude_constraint(&self) -> bool {
+        true
+    }
+
     /// see <https://www.postgresql.org/docs/13/functions-math.html>
     fn supports_factorial_operator(&self) -> bool {
         true
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
@@ -9907,50 +9907,9 @@ impl<'a> Parser<'a> {
                 ))
             }
             Token::Word(w)
-                if w.keyword == Keyword::EXCLUDE
-                    && dialect_of!(self is PostgreSqlDialect | GenericDialect) =>
+                if w.keyword == Keyword::EXCLUDE && self.dialect.supports_exclude_constraint() =>
             {
-                let index_method = if self.parse_keyword(Keyword::USING) {
-                    Some(self.parse_identifier()?)
-                } else {
-                    None
-                };
-
-                self.expect_token(&Token::LParen)?;
-                let elements = self.parse_comma_separated(|p| p.parse_exclusion_element())?;
-                self.expect_token(&Token::RParen)?;
-
-                let include = if self.parse_keyword(Keyword::INCLUDE) {
-                    self.expect_token(&Token::LParen)?;
-                    let cols = self.parse_comma_separated(|p| p.parse_identifier())?;
-                    self.expect_token(&Token::RParen)?;
-                    cols
-                } else {
-                    vec![]
-                };
-
-                let where_clause = if self.parse_keyword(Keyword::WHERE) {
-                    self.expect_token(&Token::LParen)?;
-                    let predicate = self.parse_expr()?;
-                    self.expect_token(&Token::RParen)?;
-                    Some(Box::new(predicate))
-                } else {
-                    None
-                };
-
-                let characteristics = self.parse_constraint_characteristics()?;
-
-                Ok(Some(
-                    ExclusionConstraint {
-                        name,
-                        index_method,
-                        elements,
-                        include,
-                        where_clause,
-                        characteristics,
-                    }
-                    .into(),
-                ))
+                Ok(Some(self.parse_exclude_constraint(name)?.into()))
             }
             _ => {
                 if name.is_some() {
@@ -9963,47 +9922,88 @@ 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)?;
+    /// Parse an `EXCLUDE` table constraint, with the leading `EXCLUDE` keyword
+    /// already consumed.
+    fn parse_exclude_constraint(
+        &mut self,
+        name: Option<Ident>,
+    ) -> Result<ExcludeConstraint, ParserError> {
+        let index_method = if self.parse_keyword(Keyword::USING) {
+            Some(self.parse_identifier()?)
+        } else {
+            None
+        };
 
-        self.expect_keyword_is(Keyword::WITH)?;
-        let operator = self.parse_exclusion_operator()?;
+        self.expect_token(&Token::LParen)?;
+        let elements = self.parse_comma_separated(|p| p.parse_exclude_constraint_element())?;
+        self.expect_token(&Token::RParen)?;
 
-        Ok(ExclusionElement {
-            expr,
-            operator_class,
-            order,
-            operator,
+        let include = if self.parse_keyword(Keyword::INCLUDE) {
+            self.expect_token(&Token::LParen)?;
+            let cols = self.parse_comma_separated(|p| p.parse_identifier())?;
+            self.expect_token(&Token::RParen)?;
+            cols
+        } else {
+            vec![]
+        };
+
+        let where_clause = if self.parse_keyword(Keyword::WHERE) {
+            self.expect_token(&Token::LParen)?;
+            let predicate = self.parse_expr()?;
+            self.expect_token(&Token::RParen)?;
+            Some(Box::new(predicate))
+        } else {
+            None
+        };
+
+        let characteristics = self.parse_constraint_characteristics()?;
+
+        Ok(ExcludeConstraint {
+            name,
+            index_method,
+            elements,
+            include,
+            where_clause,
+            characteristics,
         })
     }
 
+    fn parse_exclude_constraint_element(
+        &mut self,
+    ) -> Result<ExcludeConstraintElement, ParserError> {
+        // `index_elem` grammar: { col | (expr) } [ opclass ] [ ASC | DESC ] [ NULLS FIRST | LAST ].
+        // Shared with `CREATE INDEX` columns.
+        let column = self.parse_create_index_expr()?;
+        self.expect_keyword_is(Keyword::WITH)?;
+        let operator = self.parse_exclude_constraint_operator()?;
+        Ok(ExcludeConstraintElement { column, 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> {
+    fn parse_exclude_constraint_operator(
+        &mut self,
+    ) -> Result<ExcludeConstraintOperator, ParserError> {
         if self.parse_keyword(Keyword::OPERATOR) {
-            return Ok(ExclusionOperator::PgCustom(
+            return Ok(ExcludeConstraintOperator::PgCustom(
                 self.parse_pg_operator_ident_parts()?,
             ));
         }
 
+        // Reject structural delimiters (`,`, `)`, `;`, EOF) since they signal a
+        // missing operator between `WITH` and the next element / end of list.
         let operator_token = self.next_token();
-        match &operator_token.token {
-            Token::EOF | Token::RParen | Token::Comma | Token::SemiColon => {
-                self.expected("exclusion operator", operator_token)
-            }
-            _ => Ok(ExclusionOperator::Token(operator_token.token.to_string())),
+        if matches!(
+            operator_token.token,
+            Token::EOF | Token::RParen | Token::Comma | Token::SemiColon
+        ) {
+            return self.expected("exclusion operator", operator_token);
         }
+        Ok(ExcludeConstraintOperator::Token(
+            operator_token.token.to_string(),
+        ))
     }
 
     /// Parse the body of a Postgres `OPERATOR(schema.op)` form — i.e. the
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs

Original file line number	Diff line number	Diff line change
`@@ -650,7 +650,7 @@ impl Spanned for TableConstraint {`
`650`	`650`	`TableConstraint::FulltextOrSpatial(constraint) => constraint.span(),`
`651`	`651`	`TableConstraint::PrimaryKeyUsingIndex(constraint)`
`652`	`652`	`\| TableConstraint::UniqueUsingIndex(constraint) => constraint.span(),`
`653`		`- TableConstraint::Exclusion(constraint) => constraint.span(),`
	`653`	`+ TableConstraint::Exclude(constraint) => constraint.span(),`
`654`	`654`	`}`
`655`	`655`	`}`
`656`	`656`	`}`
Original file line number	Diff line number	Diff line change
`@@ -129,6 +129,10 @@ impl Dialect for GenericDialect {`
`129`	`129`	`true`
`130`	`130`	`}`
`131`	`131`
	`132`	`+ fn supports_exclude_constraint(&self) -> bool {`
	`133`	`+ true`
	`134`	`+ }`
	`135`	`+`
`132`	`136`	`fn supports_limit_comma(&self) -> bool {`
`133`	`137`	`true`
`134`	`138`	`}`