iffyio commented on code in PR #2307:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2307#discussion_r3274759652
##########
src/parser/mod.rs:
##########
@@ -10020,6 +10016,111 @@ impl<'a> Parser<'a> {
}
}
+ /// 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_token(&Token::LParen)?;
+ let elements = self.parse_comma_separated(|p|
p.parse_exclude_constraint_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(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 ].
Review Comment:
```suggestion
```
##########
tests/sqlparser_common.rs:
##########
@@ -4458,6 +4474,94 @@ fn parse_create_table_with_multiple_on_delete_fails() {
.expect_err("should have failed");
}
+#[test]
+fn parse_exclude_constraint() {
+ let dialects = all_dialects_where(|d| d.supports_exclude_constraint());
+
+ // One AST-asserting case to lock the structure of the parsed constraint;
+ // every other case below relies on `verified_stmt` round-tripping.
Review Comment:
```suggestion
```
##########
tests/sqlparser_common.rs:
##########
@@ -4458,6 +4474,94 @@ fn parse_create_table_with_multiple_on_delete_fails() {
.expect_err("should have failed");
}
+#[test]
+fn parse_exclude_constraint() {
+ let dialects = all_dialects_where(|d| d.supports_exclude_constraint());
+
+ // One AST-asserting case to lock the structure of the parsed constraint;
+ // every other case below relies on `verified_stmt` round-tripping.
+ let sql = "CREATE TABLE t (room INT, CONSTRAINT no_overlap EXCLUDE USING
gist (room WITH =))";
+ match dialects.verified_stmt(sql) {
+ Statement::CreateTable(create_table) => match
&create_table.constraints[..] {
+ [TableConstraint::Exclude(c)] => {
+ assert_eq!(c.name, Some(Ident::new("no_overlap")));
+ assert_eq!(c.index_method, Some(Ident::new("gist")));
+ assert_eq!(c.elements.len(), 1);
+ assert_eq!(
+ c.elements[0].column.column.expr,
+ Expr::Identifier(Ident::new("room"))
+ );
+ assert_eq!(c.elements[0].operator.to_string(), "=");
+ assert!(c.elements[0].column.operator_class.is_none());
+ assert!(c.include.is_empty());
+ assert!(c.where_clause.is_none());
+ assert!(c.characteristics.is_none());
+ }
+ other => panic!("expected single Exclude constraint, got
{other:?}"),
+ },
+ other => panic!("expected CreateTable, got {other:?}"),
+ }
+
+ // Round-trip a representative range of forms (`USING`/no `USING`,
+ // `INCLUDE`, `WHERE`, `DEFERRABLE`, multi-element, ordering options,
+ // operator class, function expression, schema-qualified `OPERATOR(...)`,
+ // collation, `ALTER TABLE ... ADD CONSTRAINT ... EXCLUDE`).
Review Comment:
```suggestion
```
##########
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:
it doesn't seem that this was addressed?
##########
tests/sqlparser_common.rs:
##########
@@ -2888,6 +2888,22 @@ fn parse_order_by_using_operator_invalid_cases() {
assert!(matches!(err, ParserError::ParserError(_)));
}
+#[test]
+fn parse_operator_empty_parens_rejected() {
+ // The shared `parse_pg_operator_ident_parts` helper rejects an empty
+ // `OPERATOR()` in the binary-expression path, not only in EXCLUDE
+ // constraints. Lock in that behavior.
+ let dialects = TestedDialects::new(vec![
+ Box::new(PostgreSqlDialect {}),
+ Box::new(GenericDialect {}),
+ ]);
+ let result = dialects.parse_sql_statements("SELECT a OPERATOR() b");
+ assert_eq!(
+ ParserError::ParserError("Expected: operator name, found:
)".to_string()),
+ result.unwrap_err()
+ );
+}
Review Comment:
can we move this to postgres tests instead (using pg_and_generic())?
##########
tests/sqlparser_common.rs:
##########
@@ -2888,6 +2888,22 @@ fn parse_order_by_using_operator_invalid_cases() {
assert!(matches!(err, ParserError::ParserError(_)));
}
+#[test]
+fn parse_operator_empty_parens_rejected() {
+ // The shared `parse_pg_operator_ident_parts` helper rejects an empty
+ // `OPERATOR()` in the binary-expression path, not only in EXCLUDE
+ // constraints. Lock in that behavior.
Review Comment:
```suggestion
```
--
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]
