Feature: Add IF EXISTS to REMOVE TABLE statement (#3243)

Co-authored-by: Mees Delzenne <DelSkayn@users.noreply.github.com>
Co-authored-by: Micha de Vries <micha@devrie.sh>

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?;
+		match run.get_tb(opt.ns(), opt.db(), &self.name).await {
-		// Delete the definition
+			Ok(tb) => {
-		let key = crate::key::database::tb::new(opt.ns(), opt.db(), &self.name);
+				// Delete the definition
-		run.del(key).await?;
+				let key = crate::key::database::tb::new(opt.ns(), opt.db(), &self.name);
-		// 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?;
+				// 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(())
 	}
 }

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<Self::Ok, Error> {
 		Ok(RemoveTableStatement {
 			name: self.name,
+			if_exists: self.if_exists,
 		})
 	}
 }

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());
+	}
 }

lib/src/syn/v2/lexer/keywords.rs

@@ -56,6 +56,7 @@ pub(crate) static KEYWORDS: phf::Map<UniCase<&'static str>, 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),

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") => {

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,
 		}))
 	);
 

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",

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
 //