From 334c117a486a65bc5e2b8398d467bf8ace71055e Mon Sep 17 00:00:00 2001 From: Gustavo <42685889+tavindev@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:02:07 +0000 Subject: [PATCH] Feature: Add `IF EXISTS` to `REMOVE TABLE` statement (#3243) Co-authored-by: Mees Delzenne Co-authored-by: Micha de Vries --- lib/src/sql/statements/remove/table.rs | 50 ++++++++++++------- .../value/serde/ser/statement/remove/table.rs | 5 ++ lib/src/syn/v1/stmt/remove.rs | 29 +++++++++++ lib/src/syn/v2/lexer/keywords.rs | 1 + lib/src/syn/v2/parser/stmt/remove.rs | 8 +++ lib/src/syn/v2/parser/test/stmt.rs | 1 + lib/src/syn/v2/token/keyword.rs | 1 + lib/tests/remove.rs | 32 ++++++++++++ 8 files changed, 110 insertions(+), 17 deletions(-) diff --git a/lib/src/sql/statements/remove/table.rs b/lib/src/sql/statements/remove/table.rs index f956caba..968d571e 100644 --- a/lib/src/sql/statements/remove/table.rs +++ b/lib/src/sql/statements/remove/table.rs @@ -10,10 +10,12 @@ use serde::{Deserialize, Serialize}; use std::fmt::{self, Display, Formatter}; #[derive(Clone, Debug, Default, Eq, PartialEq, PartialOrd, Serialize, Deserialize, Store, Hash)] -#[revisioned(revision = 1)] +#[revisioned(revision = 2)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct RemoveTableStatement { pub name: Ident, + #[revision(start = 2)] + pub if_exists: bool, } impl RemoveTableStatement { @@ -33,29 +35,43 @@ impl RemoveTableStatement { // Clear the cache run.clear_cache(); // Get the defined table - let tb = run.get_tb(opt.ns(), opt.db(), &self.name).await?; - // Delete the definition - let key = crate::key::database::tb::new(opt.ns(), opt.db(), &self.name); - run.del(key).await?; - // Remove the resource data - let key = crate::key::table::all::new(opt.ns(), opt.db(), &self.name); - run.delp(key, u32::MAX).await?; - // Check if this is a foreign table - if let Some(view) = &tb.view { - // Process each foreign table - for v in view.what.0.iter() { - // Save the view config - let key = crate::key::table::ft::new(opt.ns(), opt.db(), v, &self.name); + match run.get_tb(opt.ns(), opt.db(), &self.name).await { + Ok(tb) => { + // Delete the definition + let key = crate::key::database::tb::new(opt.ns(), opt.db(), &self.name); run.del(key).await?; + // Remove the resource data + let key = crate::key::table::all::new(opt.ns(), opt.db(), &self.name); + run.delp(key, u32::MAX).await?; + // Check if this is a foreign table + if let Some(view) = &tb.view { + // Process each foreign table + for v in view.what.0.iter() { + // Save the view config + let key = crate::key::table::ft::new(opt.ns(), opt.db(), v, &self.name); + run.del(key).await?; + } + } + // Ok all good + Ok(Value::None) + } + Err(err) => { + if matches!(err, Error::TbNotFound { .. }) && self.if_exists { + Ok(Value::None) + } else { + Err(err) + } } } - // Ok all good - Ok(Value::None) } } impl Display for RemoveTableStatement { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - write!(f, "REMOVE TABLE {}", self.name) + write!(f, "REMOVE TABLE {}", self.name)?; + if self.if_exists { + write!(f, " IF EXISTS")? + } + Ok(()) } } diff --git a/lib/src/sql/value/serde/ser/statement/remove/table.rs b/lib/src/sql/value/serde/ser/statement/remove/table.rs index 68ccc066..db60b6c8 100644 --- a/lib/src/sql/value/serde/ser/statement/remove/table.rs +++ b/lib/src/sql/value/serde/ser/statement/remove/table.rs @@ -36,6 +36,7 @@ impl ser::Serializer for Serializer { #[derive(Default)] pub struct SerializeRemoveTableStatement { name: Ident, + if_exists: bool, } impl serde::ser::SerializeStruct for SerializeRemoveTableStatement { @@ -50,6 +51,9 @@ impl serde::ser::SerializeStruct for SerializeRemoveTableStatement { "name" => { self.name = Ident(value.serialize(ser::string::Serializer.wrap())?); } + "if_exists" => { + self.if_exists = value.serialize(ser::primitive::bool::Serializer.wrap())? + } key => { return Err(Error::custom(format!( "unexpected field `RemoveTableStatement::{key}`" @@ -62,6 +66,7 @@ impl serde::ser::SerializeStruct for SerializeRemoveTableStatement { fn end(self) -> Result { Ok(RemoveTableStatement { name: self.name, + if_exists: self.if_exists, }) } } diff --git a/lib/src/syn/v1/stmt/remove.rs b/lib/src/syn/v1/stmt/remove.rs index 3bc8cadc..c8cbaf89 100644 --- a/lib/src/syn/v1/stmt/remove.rs +++ b/lib/src/syn/v1/stmt/remove.rs @@ -178,10 +178,16 @@ pub fn table(i: &str) -> IResult<&str, RemoveTableStatement> { let (i, _) = tag_no_case("TABLE")(i)?; let (i, _) = shouldbespace(i)?; let (i, name) = cut(ident)(i)?; + let (i, if_exists) = opt(tuple(( + shouldbespace, + tag_no_case("IF"), + cut(tuple((shouldbespace, tag_no_case("EXISTS")))), + )))(i)?; Ok(( i, RemoveTableStatement { name, + if_exists: if_exists.is_some(), }, )) } @@ -242,4 +248,27 @@ mod tests { let out = res.unwrap().1; assert_eq!("REMOVE FUNCTION fn::foo::bar::baz::bac", format!("{}", out)) } + + #[test] + fn remove_table() { + let sql = "REMOVE TABLE test"; + let res = remove(sql); + let out = res.unwrap().1; + assert_eq!("REMOVE TABLE test", format!("{}", out)) + } + + #[test] + fn remove_table_if_exists() { + let sql = "REMOVE TABLE test IF EXISTS"; + let res = remove(sql); + let out = res.unwrap().1; + assert_eq!("REMOVE TABLE test IF EXISTS", format!("{}", out)) + } + + #[test] + fn remove_table_if() { + let sql = "REMOVE TABLE test IF"; + let res = remove(sql); + assert!(res.is_err()); + } } diff --git a/lib/src/syn/v2/lexer/keywords.rs b/lib/src/syn/v2/lexer/keywords.rs index 9874a6bc..ad186a0a 100644 --- a/lib/src/syn/v2/lexer/keywords.rs +++ b/lib/src/syn/v2/lexer/keywords.rs @@ -56,6 +56,7 @@ pub(crate) static KEYWORDS: phf::Map, TokenKind> = phf_map UniCase::ascii("EVENT") => TokenKind::Keyword(Keyword::Event), UniCase::ascii("ELSE") => TokenKind::Keyword(Keyword::Else), UniCase::ascii("END") => TokenKind::Keyword(Keyword::End), + UniCase::ascii("EXISTS") => TokenKind::Keyword(Keyword::Exists), UniCase::ascii("EXPLAIN") => TokenKind::Keyword(Keyword::Explain), UniCase::ascii("false") => TokenKind::Keyword(Keyword::False), UniCase::ascii("FETCH") => TokenKind::Keyword(Keyword::Fetch), diff --git a/lib/src/syn/v2/parser/stmt/remove.rs b/lib/src/syn/v2/parser/stmt/remove.rs index 249dd698..10254f2b 100644 --- a/lib/src/syn/v2/parser/stmt/remove.rs +++ b/lib/src/syn/v2/parser/stmt/remove.rs @@ -65,8 +65,16 @@ impl Parser<'_> { } t!("TABLE") => { let name = self.next_token_value()?; + let if_exists = if self.eat(t!("IF")) { + expected!(self, t!("EXISTS")); + true + } else { + false + }; + RemoveStatement::Table(crate::sql::statements::RemoveTableStatement { name, + if_exists, }) } t!("EVENT") => { diff --git a/lib/src/syn/v2/parser/test/stmt.rs b/lib/src/syn/v2/parser/test/stmt.rs index 4e254374..8b580e97 100644 --- a/lib/src/syn/v2/parser/test/stmt.rs +++ b/lib/src/syn/v2/parser/test/stmt.rs @@ -1106,6 +1106,7 @@ fn parse_remove() { res, Statement::Remove(RemoveStatement::Table(RemoveTableStatement { name: Ident("foo".to_owned()), + if_exists: false, })) ); diff --git a/lib/src/syn/v2/token/keyword.rs b/lib/src/syn/v2/token/keyword.rs index c169f2f0..5659247e 100644 --- a/lib/src/syn/v2/token/keyword.rs +++ b/lib/src/syn/v2/token/keyword.rs @@ -68,6 +68,7 @@ keyword! { Event => "EVENT", Else => "ELSE", End => "END", + Exists => "EXISTS", Explain => "EXPLAIN", False => "false", Fetch => "FETCH", diff --git a/lib/tests/remove.rs b/lib/tests/remove.rs index 02a2f7de..59bcba77 100644 --- a/lib/tests/remove.rs +++ b/lib/tests/remove.rs @@ -129,6 +129,38 @@ async fn remove_statement_index() -> Result<(), Error> { Ok(()) } +#[tokio::test] +async fn should_error_when_remove_and_table_does_not_exist() -> Result<(), Error> { + let sql = " + REMOVE TABLE foo; + "; + let dbs = new_ds().await?; + let ses = Session::owner().with_ns("test").with_db("test"); + let res = &mut dbs.execute(sql, &ses, None).await?; + assert_eq!(res.len(), 1); + // + let tmp = res.remove(0).result.unwrap_err(); + assert!(matches!(tmp, Error::TbNotFound { .. }),); + + Ok(()) +} + +#[tokio::test] +async fn should_not_error_when_remove_if_exists() -> Result<(), Error> { + let sql = " + REMOVE TABLE foo IF EXISTS; + "; + let dbs = new_ds().await?; + let ses = Session::owner().with_ns("test").with_db("test"); + let res = &mut dbs.execute(sql, &ses, None).await?; + assert_eq!(res.len(), 1); + // + let tmp = res.remove(0).result?; + assert_eq!(tmp, Value::None); + + Ok(()) +} + // // Permissions //