Merge pull request #65 from nickolay/pr/ddl-improvements

* Rewrite parsing of `ALTER TABLE ADD CONSTRAINT`
* Support constraints in CREATE TABLE
* Change `Value::Long()` to be unsigned, use u64 consistently
* Allow trailing comma in CREATE TABLE

src/sqlast/ddl.rs

@@ -0,0 +1,86 @@
+//! AST types specific to CREATE/ALTER variants of `SQLStatement`
+//! (commonly referred to as Data Definition Language, or DDL)
+use super::{ASTNode, SQLIdent, SQLObjectName};
+
+/// An `ALTER TABLE` (`SQLStatement::SQLAlterTable`) operation
+#[derive(Debug, Clone, PartialEq)]
+pub enum AlterTableOperation {
+    /// `ADD <table_constraint>`
+    AddConstraint(TableConstraint),
+    /// TODO: implement `DROP CONSTRAINT <name>`
+    DropConstraint { name: SQLIdent },
+}
+
+impl ToString for AlterTableOperation {
+    fn to_string(&self) -> String {
+        match self {
+            AlterTableOperation::AddConstraint(c) => format!("ADD {}", c.to_string()),
+            AlterTableOperation::DropConstraint { name } => format!("DROP CONSTRAINT {}", name),
+        }
+    }
+}
+
+/// A table-level constraint, specified in a `CREATE TABLE` or an
+/// `ALTER TABLE ADD <constraint>` statement.
+#[derive(Debug, Clone, PartialEq)]
+pub enum TableConstraint {
+    /// `[ CONSTRAINT <name> ] { PRIMARY KEY | UNIQUE } (<columns>)`
+    Unique {
+        name: Option<SQLIdent>,
+        columns: Vec<SQLIdent>,
+        /// Whether this is a `PRIMARY KEY` or just a `UNIQUE` constraint
+        is_primary: bool,
+    },
+    /// A referential integrity constraint (`[ CONSTRAINT <name> ] FOREIGN KEY (<columns>)
+    /// REFERENCES <foreign_table> (<referred_columns>)`)
+    ForeignKey {
+        name: Option<SQLIdent>,
+        columns: Vec<SQLIdent>,
+        foreign_table: SQLObjectName,
+        referred_columns: Vec<SQLIdent>,
+    },
+    /// `[ CONSTRAINT <name> ] CHECK (<expr>)`
+    Check {
+        name: Option<SQLIdent>,
+        expr: Box<ASTNode>,
+    },
+}
+
+impl ToString for TableConstraint {
+    fn to_string(&self) -> String {
+        fn format_constraint_name(name: &Option<SQLIdent>) -> String {
+            name.as_ref()
+                .map(|name| format!("CONSTRAINT {} ", name))
+                .unwrap_or_default()
+        }
+        match self {
+            TableConstraint::Unique {
+                name,
+                columns,
+                is_primary,
+            } => format!(
+                "{}{} ({})",
+                format_constraint_name(name),
+                if *is_primary { "PRIMARY KEY" } else { "UNIQUE" },
+                columns.join(", ")
+            ),
+            TableConstraint::ForeignKey {
+                name,
+                columns,
+                foreign_table,
+                referred_columns,
+            } => format!(
+                "{}FOREIGN KEY ({}) REFERENCES {}({})",
+                format_constraint_name(name),
+                columns.join(", "),
+                foreign_table.to_string(),
+                referred_columns.join(", ")
+            ),
+            TableConstraint::Check { name, expr } => format!(
+                "{}CHECK ({})",
+                format_constraint_name(name),
+                expr.to_string()
+            ),
+        }
+    }
+}

src/sqlast/mod.rs

@@ -14,18 +14,18 @@
 
 //! SQL Abstract Syntax Tree (AST) types
 
+mod ddl;
 mod query;
 mod sql_operator;
 mod sqltype;
-mod table_key;
 mod value;
 
+pub use self::ddl::{AlterTableOperation, TableConstraint};
 pub use self::query::{
     Cte, Fetch, Join, JoinConstraint, JoinOperator, SQLOrderByExpr, SQLQuery, SQLSelect,
     SQLSelectItem, SQLSetExpr, SQLSetOperator, TableFactor,
 };
 pub use self::sqltype::SQLType;
-pub use self::table_key::{AlterOperation, Key, TableKey};
 pub use self::value::Value;
 
 pub use self::sql_operator::SQLOperator;
@@ -396,6 +396,7 @@ pub enum SQLStatement {
         name: SQLObjectName,
         /// Optional schema
         columns: Vec<SQLColumnDef>,
+        constraints: Vec<TableConstraint>,
         external: bool,
         file_format: Option<FileFormat>,
         location: Option<String>,
@@ -404,7 +405,7 @@ pub enum SQLStatement {
     SQLAlterTable {
         /// Table name
         name: SQLObjectName,
-        operation: AlterOperation,
+        operation: AlterTableOperation,
     },
     /// DROP TABLE
     SQLDrop {
@@ -503,21 +504,30 @@ impl ToString for SQLStatement {
             SQLStatement::SQLCreateTable {
                 name,
                 columns,
+                constraints,
                 external,
                 file_format,
                 location,
-            } if *external => format!(
-                "CREATE EXTERNAL TABLE {} ({}) STORED AS {} LOCATION '{}'",
-                name.to_string(),
-                comma_separated_string(columns),
-                file_format.as_ref().unwrap().to_string(),
-                location.as_ref().unwrap()
-            ),
-            SQLStatement::SQLCreateTable { name, columns, .. } => format!(
-                "CREATE TABLE {} ({})",
-                name.to_string(),
-                comma_separated_string(columns)
-            ),
+            } => {
+                let mut s = format!(
+                    "CREATE {}TABLE {} ({}",
+                    if *external { "EXTERNAL " } else { "" },
+                    name.to_string(),
+                    comma_separated_string(columns)
+                );
+                if !constraints.is_empty() {
+                    s += &format!(", {}", comma_separated_string(constraints));
+                }
+                s += ")";
+                if *external {
+                    s += &format!(
+                        " STORED AS {} LOCATION '{}'",
+                        file_format.as_ref().unwrap().to_string(),
+                        location.as_ref().unwrap()
+                    );
+                }
+                s
+            }
             SQLStatement::SQLAlterTable { name, operation } => {
                 format!("ALTER TABLE {} {}", name.to_string(), operation.to_string())
             }

src/sqlast/sqltype.rs

@@ -1,26 +1,26 @@
 use super::SQLObjectName;
 
-/// SQL datatypes for literals in SQL statements
+/// SQL data types
 #[derive(Debug, Clone, PartialEq)]
 pub enum SQLType {
     /// Fixed-length character type e.g. CHAR(10)
-    Char(Option<usize>),
+    Char(Option<u64>),
     /// Variable-length character type e.g. VARCHAR(10)
-    Varchar(Option<usize>),
+    Varchar(Option<u64>),
     /// Uuid type
     Uuid,
     /// Large character object e.g. CLOB(1000)
-    Clob(usize),
+    Clob(u64),
     /// Fixed-length binary type e.g. BINARY(10)
-    Binary(usize),
+    Binary(u64),
     /// Variable-length binary type e.g. VARBINARY(10)
-    Varbinary(usize),
+    Varbinary(u64),
     /// Large binary object e.g. BLOB(1000)
-    Blob(usize),
+    Blob(u64),
     /// Decimal type with optional precision and scale e.g. DECIMAL(10,2)
-    Decimal(Option<usize>, Option<usize>),
+    Decimal(Option<u64>, Option<u64>),
     /// Floating point with optional precision e.g. FLOAT(8)
-    Float(Option<usize>),
+    Float(Option<u64>),
     /// Small integer
     SmallInt,
     /// Integer
@@ -87,7 +87,7 @@ impl ToString for SQLType {
     }
 }
 
-fn format_type_with_optional_length(sql_type: &str, len: &Option<usize>) -> String {
+fn format_type_with_optional_length(sql_type: &str, len: &Option<u64>) -> String {
     let mut s = sql_type.to_string();
     if let Some(len) = len {
         s += &format!("({})", len);

src/sqlast/table_key.rs

@@ -1,61 +0,0 @@
-use super::{SQLIdent, SQLObjectName};
-
-#[derive(Debug, Clone, PartialEq)]
-pub enum AlterOperation {
-    AddConstraint(TableKey),
-    RemoveConstraint { name: SQLIdent },
-}
-
-impl ToString for AlterOperation {
-    fn to_string(&self) -> String {
-        match self {
-            AlterOperation::AddConstraint(table_key) => {
-                format!("ADD CONSTRAINT {}", table_key.to_string())
-            }
-            AlterOperation::RemoveConstraint { name } => format!("REMOVE CONSTRAINT {}", name),
-        }
-    }
-}
-
-#[derive(Debug, Clone, PartialEq)]
-pub struct Key {
-    pub name: SQLIdent,
-    pub columns: Vec<SQLIdent>,
-}
-
-#[derive(Debug, Clone, PartialEq)]
-pub enum TableKey {
-    PrimaryKey(Key),
-    UniqueKey(Key),
-    Key(Key),
-    ForeignKey {
-        key: Key,
-        foreign_table: SQLObjectName,
-        referred_columns: Vec<SQLIdent>,
-    },
-}
-
-impl ToString for TableKey {
-    fn to_string(&self) -> String {
-        match self {
-            TableKey::PrimaryKey(ref key) => {
-                format!("{} PRIMARY KEY ({})", key.name, key.columns.join(", "))
-            }
-            TableKey::UniqueKey(ref key) => {
-                format!("{} UNIQUE KEY ({})", key.name, key.columns.join(", "))
-            }
-            TableKey::Key(ref key) => format!("{} KEY ({})", key.name, key.columns.join(", ")),
-            TableKey::ForeignKey {
-                key,
-                foreign_table,
-                referred_columns,
-            } => format!(
-                "{} FOREIGN KEY ({}) REFERENCES {}({})",
-                key.name,
-                key.columns.join(", "),
-                foreign_table.to_string(),
-                referred_columns.join(", ")
-            ),
-        }
-    }
-}

src/sqlast/value.rs

@@ -1,15 +1,15 @@
-/// SQL values such as int, double, string, timestamp
+/// Primitive SQL values such as number and string
 #[derive(Debug, Clone, PartialEq)]
 pub enum Value {
-    /// Literal signed long
-    Long(i64),
-    /// Literal floating point value
+    /// Unsigned integer value
+    Long(u64),
+    /// Unsigned floating point value
     Double(f64),
     /// 'string value'
     SingleQuotedString(String),
     /// N'string value'
     NationalStringLiteral(String),
-    /// Boolean value true or false,
+    /// Boolean value true or false
     Boolean(bool),
     /// NULL value in insert statements,
     Null,

src/sqlparser.rs

@@ -347,14 +347,8 @@ impl Parser {
             let rows = if self.parse_keyword("UNBOUNDED") {
                 None
             } else {
-                let rows = self.parse_literal_int()?;
-                if rows < 0 {
-                    parser_err!(format!(
-                        "The number of rows must be non-negative, got {}",
-                        rows
-                    ))?;
-                }
-                Some(rows as u64)
+                let rows = self.parse_literal_uint()?;
+                Some(rows)
             };
             if self.parse_keyword("PRECEDING") {
                 Ok(SQLWindowFrameBound::Preceding(rows))
@@ -774,7 +768,7 @@ impl Parser {
     pub fn parse_create_external_table(&mut self) -> Result<SQLStatement, ParserError> {
         self.expect_keyword("TABLE")?;
         let table_name = self.parse_object_name()?;
-        let columns = self.parse_columns()?;
+        let (columns, constraints) = self.parse_columns()?;
         self.expect_keyword("STORED")?;
         self.expect_keyword("AS")?;
         let file_format = self.parse_identifier()?.parse::<FileFormat>()?;
@@ -785,6 +779,7 @@ impl Parser {
         Ok(SQLStatement::SQLCreateTable {
             name: table_name,
             columns,
+            constraints,
             external: true,
             file_format: Some(file_format),
             location: Some(location),
@@ -850,100 +845,120 @@ impl Parser {
     pub fn parse_create_table(&mut self) -> Result<SQLStatement, ParserError> {
         let table_name = self.parse_object_name()?;
         // parse optional column list (schema)
-        let columns = self.parse_columns()?;
+        let (columns, constraints) = self.parse_columns()?;
 
         Ok(SQLStatement::SQLCreateTable {
             name: table_name,
             columns,
+            constraints,
             external: false,
             file_format: None,
             location: None,
         })
     }
 
-    fn parse_columns(&mut self) -> Result<Vec<SQLColumnDef>, ParserError> {
+    fn parse_columns(&mut self) -> Result<(Vec<SQLColumnDef>, Vec<TableConstraint>), ParserError> {
         let mut columns = vec![];
+        let mut constraints = vec![];
         if !self.consume_token(&Token::LParen) {
-            return Ok(columns);
+            return Ok((columns, constraints));
         }
 
         loop {
-            match self.next_token() {
-                Some(Token::SQLWord(column_name)) => {
-                    let data_type = self.parse_data_type()?;
-                    let is_primary = self.parse_keywords(vec!["PRIMARY", "KEY"]);
-                    let is_unique = self.parse_keyword("UNIQUE");
-                    let default = if self.parse_keyword("DEFAULT") {
-                        let expr = self.parse_default_expr(0)?;
-                        Some(expr)
-                    } else {
-                        None
-                    };
-                    let allow_null = if self.parse_keywords(vec!["NOT", "NULL"]) {
-                        false
-                    } else {
-                        let _ = self.parse_keyword("NULL");
-                        true
-                    };
-                    debug!("default: {:?}", default);
+            if let Some(constraint) = self.parse_optional_table_constraint()? {
+                constraints.push(constraint);
+            } else if let Some(Token::SQLWord(column_name)) = self.peek_token() {
+                self.next_token();
+                let data_type = self.parse_data_type()?;
+                let is_primary = self.parse_keywords(vec!["PRIMARY", "KEY"]);
+                let is_unique = self.parse_keyword("UNIQUE");
+                let default = if self.parse_keyword("DEFAULT") {
+                    let expr = self.parse_default_expr(0)?;
+                    Some(expr)
+                } else {
+                    None
+                };
+                let allow_null = if self.parse_keywords(vec!["NOT", "NULL"]) {
+                    false
+                } else {
+                    let _ = self.parse_keyword("NULL");
+                    true
+                };
+                debug!("default: {:?}", default);
 
-                    columns.push(SQLColumnDef {
-                        name: column_name.as_sql_ident(),
-                        data_type,
-                        allow_null,
-                        is_primary,
-                        is_unique,
-                        default,
-                    });
-                    match self.next_token() {
-                        Some(Token::Comma) => {}
-                        Some(Token::RParen) => {
-                            break;
-                        }
-                        other => {
-                            return parser_err!(format!(
-                                "Expected ',' or ')' after column definition but found {:?}",
-                                other
-                            ));
-                        }
-                    }
-                }
-                unexpected => {
-                    return parser_err!(format!("Expected column name, got {:?}", unexpected));
-                }
+                columns.push(SQLColumnDef {
+                    name: column_name.as_sql_ident(),
+                    data_type,
+                    allow_null,
+                    is_primary,
+                    is_unique,
+                    default,
+                });
+            } else {
+                return self.expected("column name or constraint definition", self.peek_token());
+            }
+            let comma = self.consume_token(&Token::Comma);
+            if self.consume_token(&Token::RParen) {
+                // allow a trailing comma, even though it's not in standard
+                break;
+            } else if !comma {
+                return self.expected("',' or ')' after column definition", self.peek_token());
             }
         }
 
-        Ok(columns)
+        Ok((columns, constraints))
     }
 
-    pub fn parse_table_key(&mut self, constraint_name: SQLIdent) -> Result<TableKey, ParserError> {
-        let is_primary_key = self.parse_keywords(vec!["PRIMARY", "KEY"]);
-        let is_unique_key = self.parse_keywords(vec!["UNIQUE", "KEY"]);
-        let is_foreign_key = self.parse_keywords(vec!["FOREIGN", "KEY"]);
-        let column_names = self.parse_parenthesized_column_list(Mandatory)?;
-        let key = Key {
-            name: constraint_name,
-            columns: column_names,
-        };
-        if is_primary_key {
-            Ok(TableKey::PrimaryKey(key))
-        } else if is_unique_key {
-            Ok(TableKey::UniqueKey(key))
-        } else if is_foreign_key {
-            self.expect_keyword("REFERENCES")?;
-            let foreign_table = self.parse_object_name()?;
-            let referred_columns = self.parse_parenthesized_column_list(Mandatory)?;
-            Ok(TableKey::ForeignKey {
-                key,
-                foreign_table,
-                referred_columns,
-            })
+    pub fn parse_optional_table_constraint(
+        &mut self,
+    ) -> Result<Option<TableConstraint>, ParserError> {
+        let name = if self.parse_keyword("CONSTRAINT") {
+            Some(self.parse_identifier()?)
         } else {
-            parser_err!(format!(
-                "Expecting primary key, unique key, or foreign key, found: {:?}",
-                self.peek_token()
-            ))
+            None
+        };
+        match self.next_token() {
+            Some(Token::SQLWord(ref k)) if k.keyword == "PRIMARY" || k.keyword == "UNIQUE" => {
+                let is_primary = k.keyword == "PRIMARY";
+                if is_primary {
+                    self.expect_keyword("KEY")?;
+                }
+                let columns = self.parse_parenthesized_column_list(Mandatory)?;
+                Ok(Some(TableConstraint::Unique {
+                    name,
+                    columns,
+                    is_primary,
+                }))
+            }
+            Some(Token::SQLWord(ref k)) if k.keyword == "FOREIGN" => {
+                self.expect_keyword("KEY")?;
+                let columns = self.parse_parenthesized_column_list(Mandatory)?;
+                self.expect_keyword("REFERENCES")?;
+                let foreign_table = self.parse_object_name()?;
+                let referred_columns = self.parse_parenthesized_column_list(Mandatory)?;
+                Ok(Some(TableConstraint::ForeignKey {
+                    name,
+                    columns,
+                    foreign_table,
+                    referred_columns,
+                }))
+            }
+            Some(Token::SQLWord(ref k)) if k.keyword == "CHECK" => {
+                self.expect_token(&Token::LParen)?;
+                let expr = Box::new(self.parse_expr()?);
+                self.expect_token(&Token::RParen)?;
+                Ok(Some(TableConstraint::Check { name, expr }))
+            }
+            unexpected => {
+                if name.is_some() {
+                    self.expected("PRIMARY, UNIQUE, FOREIGN, or CHECK", unexpected)
+                } else {
+                    if unexpected.is_some() {
+                        self.prev_token();
+                    }
+                    Ok(None)
+                }
+            }
         }
     }
 
@@ -952,12 +967,10 @@ impl Parser {
         let _ = self.parse_keyword("ONLY");
         let table_name = self.parse_object_name()?;
         let operation = if self.parse_keyword("ADD") {
-            if self.parse_keyword("CONSTRAINT") {
-                let constraint_name = self.parse_identifier()?;
-                let table_key = self.parse_table_key(constraint_name)?;
-                AlterOperation::AddConstraint(table_key)
+            if let Some(constraint) = self.parse_optional_table_constraint()? {
+                AlterTableOperation::AddConstraint(constraint)
             } else {
-                return self.expected("CONSTRAINT after ADD", self.peek_token());
+                return self.expected("a constraint in ALTER TABLE .. ADD", self.peek_token());
             }
         } else {
             return self.expected("ADD after ALTER TABLE", self.peek_token());
@@ -1045,9 +1058,9 @@ impl Parser {
                     Ok(n) => Ok(Value::Double(n)),
                     Err(e) => parser_err!(format!("Could not parse '{}' as f64: {}", n, e)),
                 },
-                Token::Number(ref n) => match n.parse::<i64>() {
+                Token::Number(ref n) => match n.parse::<u64>() {
                     Ok(n) => Ok(Value::Long(n)),
-                    Err(e) => parser_err!(format!("Could not parse '{}' as i64: {}", n, e)),
+                    Err(e) => parser_err!(format!("Could not parse '{}' as u64: {}", n, e)),
                 },
                 Token::SingleQuotedString(ref s) => Ok(Value::SingleQuotedString(s.to_string())),
                 Token::NationalStringLiteral(ref s) => {
@@ -1059,13 +1072,13 @@ impl Parser {
         }
     }
 
-    /// Parse a literal integer/long
-    pub fn parse_literal_int(&mut self) -> Result<i64, ParserError> {
+    /// Parse an unsigned literal integer/long
+    pub fn parse_literal_uint(&mut self) -> Result<u64, ParserError> {
         match self.next_token() {
-            Some(Token::Number(s)) => s.parse::<i64>().map_err(|e| {
-                ParserError::ParserError(format!("Could not parse '{}' as i64: {}", s, e))
+            Some(Token::Number(s)) => s.parse::<u64>().map_err(|e| {
+                ParserError::ParserError(format!("Could not parse '{}' as u64: {}", s, e))
             }),
-            other => parser_err!(format!("Expected literal int, found {:?}", other)),
+            other => self.expected("literal int", other),
         }
     }
 
@@ -1247,17 +1260,11 @@ impl Parser {
         }
     }
 
-    pub fn parse_precision(&mut self) -> Result<usize, ParserError> {
-        //TODO: error handling
-        Ok(self.parse_optional_precision()?.unwrap())
-    }
-
-    pub fn parse_optional_precision(&mut self) -> Result<Option<usize>, ParserError> {
+    pub fn parse_optional_precision(&mut self) -> Result<Option<u64>, ParserError> {
         if self.consume_token(&Token::LParen) {
-            let n = self.parse_literal_int()?;
-            //TODO: check return value of reading rparen
+            let n = self.parse_literal_uint()?;
             self.expect_token(&Token::RParen)?;
-            Ok(Some(n as usize))
+            Ok(Some(n))
         } else {
             Ok(None)
         }
@@ -1265,16 +1272,16 @@ impl Parser {
 
     pub fn parse_optional_precision_scale(
         &mut self,
-    ) -> Result<(Option<usize>, Option<usize>), ParserError> {
+    ) -> Result<(Option<u64>, Option<u64>), ParserError> {
         if self.consume_token(&Token::LParen) {
-            let n = self.parse_literal_int()?;
+            let n = self.parse_literal_uint()?;
             let scale = if self.consume_token(&Token::Comma) {
-                Some(self.parse_literal_int()? as usize)
+                Some(self.parse_literal_uint()?)
             } else {
                 None
             };
             self.expect_token(&Token::RParen)?;
-            Ok((Some(n as usize), scale))
+            Ok((Some(n), scale))
         } else {
             Ok((None, None))
         }
@@ -1714,7 +1721,7 @@ impl Parser {
         if self.parse_keyword("ALL") {
             Ok(None)
         } else {
-            self.parse_literal_int()
+            self.parse_literal_uint()
                 .map(|n| Some(ASTNode::SQLValue(Value::Long(n))))
         }
     }
@@ -1722,7 +1729,7 @@ impl Parser {
     /// Parse an OFFSET clause
     pub fn parse_offset(&mut self) -> Result<ASTNode, ParserError> {
         let value = self
-            .parse_literal_int()
+            .parse_literal_uint()
             .map(|n| ASTNode::SQLValue(Value::Long(n)))?;
         self.expect_one_of_keywords(&["ROW", "ROWS"])?;
         Ok(value)

src/sqltokenizer.rs

@@ -29,7 +29,7 @@ use super::dialect::Dialect;
 pub enum Token {
     /// A keyword (like SELECT) or an optionally quoted SQL identifier
     SQLWord(SQLWord),
-    /// Numeric literal
+    /// An unsigned numeric literal
     Number(String),
     /// A character that could not be tokenized
     Char(char),

tests/sqlparser_common.rs

@@ -735,12 +735,14 @@ fn parse_create_table() {
         SQLStatement::SQLCreateTable {
             name,
             columns,
+            constraints,
             external: false,
             file_format: None,
             location: None,
         } => {
             assert_eq!("uk_cities", name.to_string());
             assert_eq!(3, columns.len());
+            assert!(constraints.is_empty());
 
             let c_name = &columns[0];
             assert_eq!("name", c_name.name);
@@ -761,6 +763,12 @@ fn parse_create_table() {
     }
 }
 
+#[test]
+fn parse_create_table_trailing_comma() {
+    let sql = "CREATE TABLE foo (bar int,)";
+    all_dialects().one_statement_parses_to(sql, "CREATE TABLE foo (bar int)");
+}
+
 #[test]
 fn parse_create_external_table() {
     let sql = "CREATE EXTERNAL TABLE uk_cities (\
@@ -780,12 +788,14 @@ fn parse_create_external_table() {
         SQLStatement::SQLCreateTable {
             name,
             columns,
+            constraints,
             external,
             file_format,
             location,
         } => {
             assert_eq!("uk_cities", name.to_string());
             assert_eq!(3, columns.len());
+            assert!(constraints.is_empty());
 
             let c_name = &columns[0];
             assert_eq!("name", c_name.name);
@@ -811,27 +821,52 @@ fn parse_create_external_table() {
 }
 
 #[test]
-fn parse_alter_table_constraint_primary_key() {
-    let sql = "ALTER TABLE bazaar.address \
-               ADD CONSTRAINT address_pkey PRIMARY KEY (address_id)";
-    match verified_stmt(sql) {
-        SQLStatement::SQLAlterTable { name, .. } => {
-            assert_eq!(name.to_string(), "bazaar.address");
+fn parse_alter_table_constraints() {
+    check_one("CONSTRAINT address_pkey PRIMARY KEY (address_id)");
+    check_one("CONSTRAINT uk_task UNIQUE (report_date, task_id)");
+    check_one(
+        "CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) \
+         REFERENCES public.address(address_id)",
+    );
+    check_one("CONSTRAINT ck CHECK (rtrim(ltrim(REF_CODE)) <> '')");
+
+    check_one("PRIMARY KEY (foo, bar)");
+    check_one("UNIQUE (id)");
+    check_one("FOREIGN KEY (foo, bar) REFERENCES AnotherTable(foo, bar)");
+    check_one("CHECK (end_date > start_date OR end_date IS NULL)");
+
+    fn check_one(constraint_text: &str) {
+        match verified_stmt(&format!("ALTER TABLE tab ADD {}", constraint_text)) {
+            SQLStatement::SQLAlterTable {
+                name,
+                operation: AlterTableOperation::AddConstraint(constraint),
+            } => {
+                assert_eq!("tab", name.to_string());
+                assert_eq!(constraint_text, constraint.to_string());
+            }
+            _ => unreachable!(),
         }
-        _ => unreachable!(),
+        verified_stmt(&format!("CREATE TABLE foo (id int, {})", constraint_text));
     }
 }
 
 #[test]
-fn parse_alter_table_constraint_foreign_key() {
-    let sql = "ALTER TABLE public.customer \
-        ADD CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) REFERENCES public.address(address_id)";
-    match verified_stmt(sql) {
-        SQLStatement::SQLAlterTable { name, .. } => {
-            assert_eq!(name.to_string(), "public.customer");
-        }
-        _ => unreachable!(),
-    }
+fn parse_bad_constraint() {
+    let res = parse_sql_statements("ALTER TABLE tab ADD");
+    assert_eq!(
+        ParserError::ParserError(
+            "Expected a constraint in ALTER TABLE .. ADD, found: EOF".to_string()
+        ),
+        res.unwrap_err()
+    );
+
+    let res = parse_sql_statements("CREATE TABLE tab (foo int,");
+    assert_eq!(
+        ParserError::ParserError(
+            "Expected column name or constraint definition, found: EOF".to_string()
+        ),
+        res.unwrap_err()
+    );
 }
 
 #[test]

tests/sqlparser_postgres.rs

@@ -23,12 +23,14 @@ fn parse_create_table_with_defaults() {
         SQLStatement::SQLCreateTable {
             name,
             columns,
+            constraints,
             external: false,
             file_format: None,
             location: None,
         } => {
             assert_eq!("public.customer", name.to_string());
             assert_eq!(10, columns.len());
+            assert!(constraints.is_empty());
 
             let c_name = &columns[0];
             assert_eq!("customer_id", c_name.name);
@@ -69,11 +71,13 @@ fn parse_create_table_from_pg_dump() {
         SQLStatement::SQLCreateTable {
             name,
             columns,
+            constraints,
             external: false,
             file_format: None,
             location: None,
         } => {
             assert_eq!("public.customer", name.to_string());
+            assert!(constraints.is_empty());
 
             let c_customer_id = &columns[0];
             assert_eq!("customer_id", c_customer_id.name);
@@ -130,11 +134,13 @@ fn parse_create_table_with_inherit() {
         SQLStatement::SQLCreateTable {
             name,
             columns,
+            constraints,
             external: false,
             file_format: None,
             location: None,
         } => {
             assert_eq!("bazaar.settings", name.to_string());
+            assert!(constraints.is_empty());
 
             let c_name = &columns[0];
             assert_eq!("settings_id", c_name.name);