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