Repository: gluesql/gluesql
Pull request #68 (2020-09-13 05:59:35)

support if not exists for create table and if exist for drop table
support #53 

- check if the table exists when creating a table
- check if the table does not exist when dropping a table
- support if not exists for creating a table
- support if exists for dropping a table

leoppro: I'm sorry I've been busy at work and haven't been able to update my PR, I will update it soon.
panarch: Please don't feel sorry at all, I hope you enjoy writing codes here. It's fully ok because it is not your primary work.
If there exists something blocks other issues, then I will request you something.
But in the case of this issue, it's independent from any other ones so you can simply spend time by only following your own schedule.
leoppro: > In this code, we encounter StoreError::SchemaAlreadyExists, but in specific condition if_not_exists then it transforms into Ok.
It can have some design issues, Error is better to be handled as "Terminal" state, not "Non-terminal" one.
But here, the role of "Error" is expanded to "Non-terminal" and I think we'd better to keep as narrow as possible, keeping error's role as terminal.
So, once we see the error then we should return, not changing into Ok.

hi @panarch , I don't quite understand your first comment, how should I handle the error?

Patch:
diff --git a/src/executor/execute.rs b/src/executor/execute.rs
index 99a4e314..9e7e9601 100644
--- a/src/executor/execute.rs
+++ b/src/executor/execute.rs
@@ -1,20 +1,21 @@
-use serde::Serialize;
 use std::fmt::Debug;
-use thiserror::Error;
 
+use serde::Serialize;
 #[cfg(feature = "alter-table")]
 use sqlparser::ast::AlterTableOperation;
 use sqlparser::ast::{ObjectType, SetExpr, Statement, Values};
+use thiserror::Error;
 
-use super::fetch::{fetch, fetch_columns};
-use super::filter::Filter;
-use super::select::select_with_aliases;
-use super::update::Update;
 use crate::data::{get_name, Row, Schema};
 use crate::parse::Query;
 use crate::result::{MutResult, Result};
 use crate::store::{AlterTable, Store, StoreMut};
 
+use super::fetch::{fetch, fetch_columns};
+use super::filter::Filter;
+use super::select::select_with_aliases;
+use super::update::Update;
+
 #[derive(Error, Serialize, Debug, PartialEq)]
 pub enum ExecuteError {
     #[error("query not supported")]
@@ -29,6 +30,11 @@ pub enum ExecuteError {
     #[cfg(feature = "alter-table")]
     #[error("unsupported alter table operation: {0}")]
     UnsupportedAlterTableOperation(String),
+
+    #[error("TableNotExists")]
+    TableNotExists,
+    #[error("TableAlreadyExists")]
+    TableAlreadyExists,
 }
 
 #[derive(Serialize, Debug, PartialEq)]
@@ -60,9 +66,21 @@ pub fn execute<T: 'static + Debug, U: Store<T> + StoreMut<T> + AlterTable>(
     };
 
     match prepared {
-        Prepared::Create(schema) => storage
-            .insert_schema(&schema)
-            .map(|(storage, _)| (storage, Payload::Create)),
+        Prepared::Create {
+            schema,
+            if_not_exists,
+        } => {
+            if storage.fetch_schema(&schema.table_name).is_ok() {
+                return if if_not_exists {
+                    Ok((storage, Payload::Create))
+                } else {
+                    Err((storage, ExecuteError::TableAlreadyExists.into()))
+                };
+            }
+            storage
+                .insert_schema(&schema)
+                .map(|(storage, _)| (storage, Payload::Create))
+        }
         Prepared::Insert(table_name, rows) => rows
             .into_iter()
             .try_fold((storage, 0), |(storage, num), row| {
@@ -96,8 +114,23 @@ pub fn execute<T: 'static + Debug, U: Store<T> + StoreMut<T> + AlterTable>(
 
             Ok((storage, Payload::Update(num_rows)))
         }
-        Prepared::DropTable(table_names) => {
-            let storage = table_names
+        Prepared::DropTable {
+            schema_names,
+            if_exists,
+        } => {
+            let storage = match if_exists {
+                true => Ok(storage),
+                false => schema_names
+                    .iter()
+                    .try_fold(storage, |storage, table_name| {
+                        if storage.fetch_schema(table_name).is_ok() {
+                            Ok(storage)
+                        } else {
+                            Err((storage, ExecuteError::TableNotExists.into()))
+                        }
+                    }),
+            }?;
+            let storage = schema_names
                 .iter()
                 .try_fold(storage, |storage, table_name| {
                     let (storage, _) = storage.delete_schema(table_name)?;
@@ -143,7 +176,10 @@ pub fn execute<T: 'static + Debug, U: Store<T> + StoreMut<T> + AlterTable>(
 }
 
 enum Prepared<'a, T> {
-    Create(Schema),
+    Create {
+        schema: Schema,
+        if_not_exists: bool,
+    },
     Insert(&'a str, Vec<Row>),
     Delete(Vec<T>),
     Update(Vec<(T, Row)>),
@@ -151,7 +187,10 @@ enum Prepared<'a, T> {
         aliases: Vec<String>,
         rows: Vec<Row>,
     },
-    DropTable(Vec<&'a str>),
+    DropTable {
+        schema_names: Vec<&'a str>,
+        if_exists: bool,
+    },
 
     #[cfg(feature = "alter-table")]
     AlterTable(&'a str, &'a AlterTableOperation),
@@ -162,13 +201,21 @@ fn prepare<'a, T: 'static + Debug>(
     sql_query: &'a Statement,
 ) -> Result<Prepared<'a, T>> {
     match sql_query {
-        Statement::CreateTable { name, columns, .. } => {
+        Statement::CreateTable {
+            name,
+            columns,
+            if_not_exists,
+            ..
+        } => {
             let schema = Schema {
                 table_name: get_name(name)?.clone(),
                 column_defs: columns.clone(),
             };
 
-            Ok(Prepared::Create(schema))
+            Ok(Prepared::Create {
+                schema,
+                if_not_exists: *if_not_exists,
+            })
         }
         Statement::Query(query) => {
             let (aliases, rows) = select_with_aliases(storage, &query, None, true)?;
@@ -234,7 +281,10 @@ fn prepare<'a, T: 'static + Debug>(
             Ok(Prepared::Delete(rows))
         }
         Statement::Drop {
-            object_type, names, ..
+            object_type,
+            names,
+            if_exists,
+            ..
         } => {
             if object_type != &ObjectType::Table {
                 return Err(ExecuteError::DropTypeNotSupported.into());
@@ -245,7 +295,10 @@ fn prepare<'a, T: 'static + Debug>(
                 .map(|name| get_name(name).map(|n| n.as_str()))
                 .collect::<Result<_>>()?;
 
-            Ok(Prepared::DropTable(names))
+            Ok(Prepared::DropTable {
+                schema_names: names,
+                if_exists: *if_exists,
+            })
         }
 
         #[cfg(feature = "alter-table")]


Test patch:
diff --git a/src/tests/create_table.rs b/src/tests/create_table.rs
new file mode 100644
index 00000000..a8755a4c
--- /dev/null
+++ b/src/tests/create_table.rs
@@ -0,0 +1,47 @@
+use crate::*;
+
+pub fn create_table(mut tester: impl tests::Tester) {
+    let mut run = |sql| tester.run(sql);
+
+    let test_cases = vec![
+        (
+            r#"
+        CREATE TABLE CreateTable1 (
+            id INTEGER NULL,
+            num INTEGER,
+            name TEXT
+        )"#,
+            Ok(Payload::Create),
+        ),
+        (
+            r#"
+        CREATE TABLE CreateTable1 (
+            id INTEGER NULL,
+            num INTEGER,
+            name TEXT
+        )"#,
+            Err(ExecuteError::TableAlreadyExists.into()),
+        ),
+        (
+            r#"
+        CREATE TABLE IF NOT EXISTS CreateTable2 (
+            id INTEGER NULL,
+            num INTEGER,
+            name TEXT
+        )"#,
+            Ok(Payload::Create),
+        ),
+        (
+            r#"
+        CREATE TABLE IF NOT EXISTS CreateTable2 (
+            id INTEGER NULL,
+            num INTEGER,
+            name TEXT
+        )"#,
+            Ok(Payload::Create),
+        ),
+    ];
+    test_cases
+        .into_iter()
+        .for_each(|(sql, expected)| assert_eq!(expected, run(sql)));
+}
diff --git a/src/tests/drop_table.rs b/src/tests/drop_table.rs
index eb84b765..a27b79fc 100644
--- a/src/tests/drop_table.rs
+++ b/src/tests/drop_table.rs
@@ -28,6 +28,21 @@ CREATE TABLE DropTable (
             )),
         ),
         ("DROP TABLE DropTable;", Ok(Payload::DropTable)),
+        (
+            "DROP TABLE DropTable;",
+            Err(ExecuteError::TableNotExists.into()),
+        ),
+        (
+            r#"
+CREATE TABLE DropTable (
+    id INT,
+    num INT,
+    name TEXT
+)"#,
+            Ok(Payload::Create),
+        ),
+        ("DROP TABLE IF EXISTS DropTable;", Ok(Payload::DropTable)),
+        ("DROP TABLE IF EXISTS DropTable;", Ok(Payload::DropTable)),
         (
             "SELECT id, num, name FROM DropTable;",
             Err(StoreError::SchemaNotFound.into()),
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index 7779f6f5..ebc1301a 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -4,6 +4,7 @@ pub mod alter_table;
 pub mod arithmetic;
 pub mod basic;
 pub mod blend;
+pub mod create_table;
 pub mod default;
 pub mod drop_table;
 pub mod error;
@@ -58,6 +59,7 @@ macro_rules! generate_tests {
         glue!(arithmetic, arithmetic::arithmetic);
         glue!(arithmetic_blend, arithmetic::blend);
         glue!(blend, blend::blend);
+        glue!(create_table, create_table::create_table);
         glue!(drop_table, drop_table::drop_table);
         glue!(error, error::error);
         glue!(join, join::join);