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


##########
src/parser/mod.rs:
##########
@@ -5894,6 +5896,47 @@ impl<'a> Parser<'a> {
         Ok(owner)
     }
 
+    /// ```sql
+    ///     CREATE DOMAIN name [ AS ] data_type
+    ///             [ COLLATE collation ]
+    ///             [ DEFAULT expression ]
+    ///             [ domain_constraint [ ... ] ]
+    ///     
+    ///         where domain_constraint is:
+    ///     
+    ///         [ CONSTRAINT constraint_name ]
+    ///         { NOT NULL | NULL | CHECK (expression) }
+    /// ```
+    ///
+    /// [PostgreSQL 
Documentation](https://www.postgresql.org/docs/current/sql-createdomain.html)

Review Comment:
   same here regarding duplicating the doc comment, we can only link to 
`[Statement::CreateDomain]` which readers can follow for more context around 
the statement if needed.



##########
src/parser/mod.rs:
##########
@@ -5894,6 +5896,47 @@ impl<'a> Parser<'a> {
         Ok(owner)
     }
 
+    /// ```sql
+    ///     CREATE DOMAIN name [ AS ] data_type
+    ///             [ COLLATE collation ]
+    ///             [ DEFAULT expression ]
+    ///             [ domain_constraint [ ... ] ]
+    ///     
+    ///         where domain_constraint is:
+    ///     
+    ///         [ CONSTRAINT constraint_name ]
+    ///         { NOT NULL | NULL | CHECK (expression) }
+    /// ```
+    ///
+    /// [PostgreSQL 
Documentation](https://www.postgresql.org/docs/current/sql-createdomain.html)
+    pub fn parse_create_domain(&mut self) -> Result<Statement, ParserError> {

Review Comment:
   ```suggestion
       fn parse_create_domain(&mut self) -> Result<Statement, ParserError> {
   ```



##########
src/ast/mod.rs:
##########
@@ -3968,6 +3968,19 @@ pub enum Statement {
         owned_by: Option<ObjectName>,
     },
     /// ```sql
+    /// CREATE DOMAIN name [ AS ] data_type
+    ///         [ COLLATE collation ]
+    ///         [ DEFAULT expression ]
+    ///         [ domain_constraint [ ... ] ]
+    ///
+    ///     where domain_constraint is:
+    ///
+    ///     [ CONSTRAINT constraint_name ]
+    ///     { NOT NULL | NULL | CHECK (expression) }
+    /// ```

Review Comment:
   Instead of duplicating this doc comment, maybe it would suffice to only 
mention that this is a `CREATE DOMAIN` statement, [for 
example](https://github.com/apache/datafusion-sqlparser-rs/blob/d4868ba1316419ef3dc79942eaf1eaf662be359c/src/ast/mod.rs#L2980)



##########
tests/sqlparser_postgres.rs:
##########
@@ -5080,6 +5080,111 @@ fn test_escaped_string_literal() {
     }
 }
 
+#[test]
+fn parse_create_domain() {
+    let sql1 = "CREATE DOMAIN my_domain AS INTEGER CHECK (VALUE > 0)";
+    let expected = Statement::CreateDomain(CreateDomain {
+        name: ObjectName::from(vec![Ident::new("my_domain")]),
+        data_type: DataType::Integer(None),
+        collation: None,
+        default: None,
+        constraints: vec![TableConstraint::Check {
+            name: None,
+            expr: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Identifier(Ident::new("VALUE"))),
+                op: BinaryOperator::Gt,
+                right: Box::new(Expr::Value(test_utils::number("0").into())),
+            }),
+        }],
+    });
+
+    assert_eq!(pg().verified_stmt(sql1), expected);
+}
+
+#[test]
+fn parse_create_domain_with_collation() {
+    let sql2 = "CREATE DOMAIN my_domain AS INTEGER COLLATE \"en_US\" CHECK 
(VALUE > 0)";
+    let expected = Statement::CreateDomain(CreateDomain {
+        name: ObjectName::from(vec![Ident::new("my_domain")]),
+        data_type: DataType::Integer(None),
+        collation: Some(Ident::with_quote('"', "en_US")),
+        default: None,
+        constraints: vec![TableConstraint::Check {
+            name: None,
+            expr: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Identifier(Ident::new("VALUE"))),
+                op: BinaryOperator::Gt,
+                right: Box::new(Expr::Value(test_utils::number("0").into())),
+            }),
+        }],
+    });
+
+    assert_eq!(pg().verified_stmt(sql2), expected);
+}
+
+#[test]
+fn parse_create_domain_with_default() {
+    let sql3 = "CREATE DOMAIN my_domain AS INTEGER DEFAULT 1 CHECK (VALUE > 
0)";
+    let expected = Statement::CreateDomain(CreateDomain {
+        name: ObjectName::from(vec![Ident::new("my_domain")]),
+        data_type: DataType::Integer(None),
+        collation: None,
+        default: Some(Expr::Value(test_utils::number("1").into())),
+        constraints: vec![TableConstraint::Check {
+            name: None,
+            expr: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Identifier(Ident::new("VALUE"))),
+                op: BinaryOperator::Gt,
+                right: Box::new(Expr::Value(test_utils::number("0").into())),
+            }),
+        }],
+    });
+
+    assert_eq!(pg().verified_stmt(sql3), expected);
+}
+
+#[test]
+fn parse_create_domain_with_default_and_collation() {
+    let sql4 = "CREATE DOMAIN my_domain AS INTEGER COLLATE \"en_US\" DEFAULT 1 
CHECK (VALUE > 0)";
+    let expected = Statement::CreateDomain(CreateDomain {
+        name: ObjectName::from(vec![Ident::new("my_domain")]),
+        data_type: DataType::Integer(None),
+        collation: Some(Ident::with_quote('"', "en_US")),
+        default: Some(Expr::Value(test_utils::number("1").into())),
+        constraints: vec![TableConstraint::Check {
+            name: None,
+            expr: Box::new(Expr::BinaryOp {
+                left: Box::new(Expr::Identifier(Ident::new("VALUE"))),
+                op: BinaryOperator::Gt,
+                right: Box::new(Expr::Value(test_utils::number("0").into())),
+            }),
+        }],
+    });
+
+    assert_eq!(pg().verified_stmt(sql4), expected);
+}
+
+#[test]
+fn parse_create_domain_with_named_constraint() {

Review Comment:
   Can we merge the test cases into a single test covering this feature, vs 
having one test per test case? Also we can consider relying only on 
`verified_stmt` for most of the test cases since its usually verbose and harder 
to maintain tests that assert the full AST



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to