Replace ColumnOption::{Null, NotNull} to ColumnDef->nullable field (#986

) Remove ColumnOption::{Null, NotNull} Add nullable field to ColumnDef to replace {Null, NotNull} options.
gluesql · Nov 8, 2022 · 6d147f5 · 6d147f5
1 parent 74eb4e7
commit 6d147f5
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 57 deletions.
diff --git a/core/src/ast/ddl.rs b/core/src/ast/ddl.rs
@@ -26,15 +26,12 @@ pub enum AlterTableOperation {
 pub struct ColumnDef {
     pub name: String,
     pub data_type: DataType,
+    pub nullable: bool,
     pub options: Vec<ColumnOption>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum ColumnOption {
-    /// `NULL`
-    Null,
-    /// `NOT NULL`
-    NotNull,
     /// `DEFAULT <restricted-expr>`
     Default(Expr),
     /// `{ PRIMARY KEY | UNIQUE }`
@@ -70,16 +67,22 @@ impl ToSql for ColumnDef {
         let ColumnDef {
             name,
             data_type,
+            nullable,
             options,
         } = self;
         {
+            let nullable = match nullable {
+                true => "NULL",
+                false => "NOT NULL",
+            };
+
             let options = options
                 .iter()
                 .map(|option| option.to_sql())
                 .collect::<Vec<_>>()
                 .join(" ");
 
-            format!("{name} {data_type} {options}")
+            format!("{name} {data_type} {nullable} {options}")
                 .trim_end()
                 .to_owned()
         }
@@ -89,8 +92,6 @@ impl ToSql for ColumnDef {
 impl ToSql for ColumnOption {
     fn to_sql(&self) -> String {
         match self {
-            ColumnOption::Null => "NULL".to_owned(),
-            ColumnOption::NotNull => "NOT NULL".to_owned(),
             ColumnOption::Default(expr) => format!("DEFAULT {}", expr.to_sql()),
             ColumnOption::Unique { is_primary } => match is_primary {
                 true => "PRIMARY KEY".to_owned(),
@@ -107,10 +108,11 @@ mod tests {
     #[test]
     fn to_sql_column_def() {
         assert_eq!(
-            "name TEXT UNIQUE",
+            "name TEXT NOT NULL UNIQUE",
             ColumnDef {
                 name: "name".to_owned(),
                 data_type: DataType::Text,
+                nullable: false,
                 options: vec![ColumnOption::Unique { is_primary: false }]
             }
             .to_sql()
@@ -121,7 +123,8 @@ mod tests {
             ColumnDef {
                 name: "accepted".to_owned(),
                 data_type: DataType::Boolean,
-                options: vec![ColumnOption::Null]
+                nullable: true,
+                options: Vec::new()
             }
             .to_sql()
         );
@@ -131,19 +134,18 @@ mod tests {
             ColumnDef {
                 name: "id".to_owned(),
                 data_type: DataType::Int,
-                options: vec![
-                    ColumnOption::NotNull,
-                    ColumnOption::Unique { is_primary: true }
-                ]
+                nullable: false,
+                options: vec![ColumnOption::Unique { is_primary: true }]
             }
             .to_sql()
         );
 
         assert_eq!(
-            "accepted BOOLEAN DEFAULT FALSE",
+            "accepted BOOLEAN NOT NULL DEFAULT FALSE",
             ColumnDef {
                 name: "accepted".to_owned(),
                 data_type: DataType::Boolean,
+                nullable: false,
                 options: vec![ColumnOption::Default(Expr::Literal(AstLiteral::Boolean(
                     false
                 )))]

diff --git a/core/src/ast/mod.rs b/core/src/ast/mod.rs
@@ -361,24 +361,27 @@ mod tests {
         );
 
         assert_eq!(
-            "CREATE TABLE Foo (id INT, num INT NULL, name TEXT);",
+            "CREATE TABLE Foo (id INT NOT NULL, num INT NULL, name TEXT NOT NULL);",
             Statement::CreateTable {
                 if_not_exists: false,
                 name: "Foo".into(),
                 columns: vec![
                     ColumnDef {
                         name: "id".to_owned(),
                         data_type: DataType::Int,
+                        nullable: false,
                         options: vec![]
                     },
                     ColumnDef {
                         name: "num".to_owned(),
                         data_type: DataType::Int,
-                        options: vec![ColumnOption::Null]
+                        nullable: true,
+                        options: Vec::new()
                     },
                     ColumnDef {
                         name: "name".to_owned(),
                         data_type: DataType::Text,
+                        nullable: false,
                         options: vec![]
                     }
                 ],
@@ -451,13 +454,14 @@ mod tests {
     #[cfg(feature = "alter-table")]
     fn to_sql_alter_table() {
         assert_eq!(
-            "ALTER TABLE Foo ADD COLUMN amount INT DEFAULT 10;",
+            "ALTER TABLE Foo ADD COLUMN amount INT NOT NULL DEFAULT 10;",
             Statement::AlterTable {
                 name: "Foo".into(),
                 operation: AlterTableOperation::AddColumn {
                     column_def: ColumnDef {
                         name: "amount".to_owned(),
                         data_type: DataType::Int,
+                        nullable: false,
                         options: vec![ColumnOption::Default(Expr::Literal(AstLiteral::Number(
                             BigDecimal::from_str("10").unwrap()
                         )))]

diff --git a/core/src/data/row.rs b/core/src/data/row.rs
@@ -77,6 +77,7 @@ impl Row {
                 let ColumnDef {
                     name: def_name,
                     data_type,
+                    nullable,
                     ..
                 } = column_def;
 
@@ -85,11 +86,9 @@ impl Row {
                     .find(|(name, _)| name == &def_name)
                     .map(|(_, value)| value);
 
-                let nullable = column_def.is_nullable();
-
                 match (value, column_def.get_default(), nullable) {
                     (Some(&expr), _, _) | (None, Some(expr), _) => {
-                        evaluate_stateless(None, expr)?.try_into_value(data_type, nullable)
+                        evaluate_stateless(None, expr)?.try_into_value(data_type, *nullable)
                     }
                     (None, None, true) => Ok(Value::Null),
                     (None, None, false) => {
@@ -112,11 +111,14 @@ impl Row {
             });
 
         for (value, column_def) in items {
-            let ColumnDef { data_type, .. } = column_def;
-            let nullable = column_def.is_nullable();
+            let ColumnDef {
+                data_type,
+                nullable,
+                ..
+            } = column_def;
 
             value.validate_type(data_type)?;
-            value.validate_null(nullable)?;
+            value.validate_null(*nullable)?;
         }
 
         Ok(())

diff --git a/core/src/data/schema.rs b/core/src/data/schema.rs
@@ -62,12 +62,6 @@ impl Schema {
 }
 
 impl ColumnDef {
-    pub fn is_nullable(&self) -> bool {
-        self.options
-            .iter()
-            .any(|option| option == &ColumnOption::Null)
-    }
-
     pub fn get_default(&self) -> Option<&Expr> {
         self.options.iter().find_map(|option| match option {
             ColumnOption::Default(expr) => Some(expr),
@@ -94,17 +88,16 @@ mod tests {
                 ColumnDef {
                     name: "id".to_owned(),
                     data_type: DataType::Int,
+                    nullable: false,
                     options: Vec::new(),
                 },
                 ColumnDef {
                     name: "name".to_owned(),
                     data_type: DataType::Text,
-                    options: vec![
-                        ColumnOption::Null,
-                        ColumnOption::Default(Expr::Literal(AstLiteral::QuotedString(
-                            "glue".to_owned(),
-                        ))),
-                    ],
+                    nullable: true,
+                    options: vec![ColumnOption::Default(Expr::Literal(
+                        AstLiteral::QuotedString("glue".to_owned()),
+                    ))],
                 },
             ],
             indexes: Vec::new(),
@@ -113,7 +106,7 @@ mod tests {
 
         assert_eq!(
             schema.to_ddl(),
-            "CREATE TABLE User (id INT, name TEXT NULL DEFAULT 'glue');"
+            "CREATE TABLE User (id INT NOT NULL, name TEXT NULL DEFAULT 'glue');"
         )
     }
 
@@ -124,13 +117,17 @@ mod tests {
             column_defs: vec![ColumnDef {
                 name: "id".to_owned(),
                 data_type: DataType::Int,
+                nullable: false,
                 options: vec![ColumnOption::Unique { is_primary: true }],
             }],
             indexes: Vec::new(),
             created: Utc::now().naive_utc(),
         };
 
-        assert_eq!(schema.to_ddl(), "CREATE TABLE User (id INT PRIMARY KEY);");
+        assert_eq!(
+            schema.to_ddl(),
+            "CREATE TABLE User (id INT NOT NULL PRIMARY KEY);"
+        );
     }
 
     #[test]
@@ -141,11 +138,13 @@ mod tests {
                 ColumnDef {
                     name: "id".to_owned(),
                     data_type: DataType::Int,
+                    nullable: false,
                     options: Vec::new(),
                 },
                 ColumnDef {
                     name: "name".to_owned(),
                     data_type: DataType::Text,
+                    nullable: false,
                     options: Vec::new(),
                 },
             ],
@@ -168,7 +167,7 @@ mod tests {
 
         assert_eq!(
             schema.to_ddl(),
-            "CREATE TABLE User (id INT, name TEXT);
+            "CREATE TABLE User (id INT NOT NULL, name TEXT NOT NULL);
 CREATE INDEX User_id ON User (id);
 CREATE INDEX User_name ON User (name);"
         );

diff --git a/core/src/executor/alter/table.rs b/core/src/executor/alter/table.rs
@@ -1,7 +1,7 @@
 use {
     super::{validate, AlterError},
     crate::{
-        ast::{ColumnDef, ColumnOption, Query, SetExpr, TableFactor, Values},
+        ast::{ColumnDef, Query, SetExpr, TableFactor, Values},
         data::{Schema, TableError},
         executor::{evaluate_stateless, select::select},
         prelude::{DataType, Value},
@@ -42,7 +42,8 @@ pub async fn create_table<T: GStore + GStoreMut>(
                         let column_def = ColumnDef {
                             name: "N".into(),
                             data_type: DataType::Int,
-                            options: vec![ColumnOption::NotNull],
+                            nullable: false,
+                            options: Vec::new(),
                         };
 
                         vec![column_def]
@@ -91,7 +92,8 @@ pub async fn create_table<T: GStore + GStoreMut>(
                         .map(|(i, data_type)| ColumnDef {
                             name: format!("column{}", i + 1),
                             data_type,
-                            options: vec![ColumnOption::Null],
+                            nullable: true,
+                            options: Vec::new(),
                         })
                         .collect::<Vec<_>>();
 

diff --git a/core/src/executor/update.rs b/core/src/executor/update.rs
@@ -80,8 +80,11 @@ impl<'a> Update<'a> {
             None => Ok(None),
             Some(assignment) => {
                 let Assignment { value, .. } = &assignment;
-                let ColumnDef { data_type, .. } = column_def;
-                let nullable = column_def.is_nullable();
+                let ColumnDef {
+                    data_type,
+                    nullable,
+                    ..
+                } = column_def;
 
                 let value = match evaluate(self.storage, context, None, value).await? {
                     Evaluated::Literal(v) => Value::try_from_literal(data_type, &v)?,
@@ -91,7 +94,7 @@ impl<'a> Update<'a> {
                     }
                 };
 
-                value.validate_null(nullable)?;
+                value.validate_null(*nullable)?;
 
                 Ok(Some(value))
             }

diff --git a/core/src/plan/mock.rs b/core/src/plan/mock.rs
@@ -181,6 +181,7 @@ mod tests {
                 &ColumnDef {
                     name: "new_col".to_owned(),
                     data_type: DataType::Boolean,
+                    nullable: false,
                     options: Vec::new(),
                 },
             ));

diff --git a/core/src/translate/ddl.rs b/core/src/translate/ddl.rs
@@ -59,9 +59,14 @@ pub fn translate_column_def(sql_column_def: &SqlColumnDef) -> Result<ColumnDef>
         ..
     } = sql_column_def;
 
+    let nullable = options
+        .iter()
+        .any(|SqlColumnOptionDef { option, .. }| option == &SqlColumnOption::Null);
+
     Ok(ColumnDef {
         name: name.value.to_owned(),
         data_type: translate_data_type(data_type)?,
+        nullable,
         options: options
             .iter()
             .map(translate_column_option_def)
@@ -82,17 +87,13 @@ fn translate_column_option_def(
     let SqlColumnOptionDef { option, .. } = sql_column_option_def;
 
     let option = match option {
-        SqlColumnOption::Null => Ok(ColumnOption::Null),
-        SqlColumnOption::NotNull => Ok(ColumnOption::NotNull),
+        SqlColumnOption::Null | SqlColumnOption::NotNull => return Ok(Vec::new()),
         SqlColumnOption::Default(expr) => translate_expr(expr).map(ColumnOption::Default),
         SqlColumnOption::Unique { is_primary } if !is_primary => {
             Ok(ColumnOption::Unique { is_primary: false })
         }
         SqlColumnOption::Unique { .. } => {
-            return Ok(vec![
-                ColumnOption::Unique { is_primary: true },
-                ColumnOption::NotNull,
-            ]);
+            return Ok(vec![ColumnOption::Unique { is_primary: true }]);
         }
         _ => Err(TranslateError::UnsupportedColumnOption(option.to_string()).into()),
     }?;

diff --git a/storages/memory-storage/src/alter_table.rs b/storages/memory-storage/src/alter_table.rs
@@ -63,14 +63,18 @@ impl MemoryStorage {
             return Err(AlterTableError::AddingColumnAlreadyExists(adding_column).into());
         }
 
-        let ColumnDef { data_type, .. } = column_def;
-        let nullable = column_def.is_nullable();
+        let ColumnDef {
+            data_type,
+            nullable,
+            ..
+        } = column_def;
+
         let default = column_def.get_default();
         let value = match (default, nullable) {
             (Some(expr), _) => {
                 let evaluated = gluesql_core::executor::evaluate_stateless(None, expr)?;
 
-                evaluated.try_into_value(data_type, nullable)?
+                evaluated.try_into_value(data_type, *nullable)?
             }
             (None, true) => Value::Null,
             (None, false) => {