fix: clean up matches and remove panic

Signed-off-by: callum-ryan <[email protected]>
apache · Jul 31, 2024 · 6f33fae · 6f33fae
1 parent dc36b83
commit 6f33fae
Showing 1 changed file with 39 additions and 33 deletions.
diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs
@@ -534,7 +534,24 @@ impl Catalog for SqlCatalog {
         )
         .await?;
 
-        Ok(())
+        let table_existence = self.table_exists(identifier).await;
+
+        match table_existence {
+            Ok(res) => {
+                if res {
+                    Err(Error::new(
+                        ErrorKind::Unexpected,
+                        "drop table was not successful",
+                    ))
+                } else {
+                    Ok(())
+                }
+            }
+            _ => Err(Error::new(
+                ErrorKind::Unexpected,
+                "drop table was not successful",
+            )),
+        }
     }
 
     async fn load_table(&self, identifier: &TableIdent) -> Result<Table> {
@@ -560,7 +577,6 @@ impl Catalog for SqlCatalog {
                     vec![Some(&catalog_name), Some(&namespace), Some(&name)],
                 )
                 .await?;
-            assert_eq!(row.len(), 1, "expected only one row from load_table query");
             let row = query_map(&row[0]).map_err(from_sqlx_error)?;
             row.metadata_location
         };
@@ -641,30 +657,25 @@ impl Catalog for SqlCatalog {
         let src_table_exist = self.table_exists(src).await;
         let dst_table_exist = self.table_exists(dest).await;
 
-        match src_table_exist {
-            Ok(res) => {
-                if res {
-                    match dst_table_exist {
-                        Ok(dst_res) => {
-                            if dst_res {
-                                Err(Error::new(
-                                    ErrorKind::Unexpected,
-                                    "failed to rename table as destination already exists",
-                                ))
-                            } else {
-                                Ok(())
-                            }
-                        }
-                        Err(_) => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
-                    }
-                } else {
+        match (src_table_exist, dst_table_exist) {
+            (Ok(src_res), Ok(dst_res)) => {
+                if src_res && !dst_res {
+                    Ok(())
+                } else if src_res && dst_res {
+                    Err(Error::new(
+                        ErrorKind::Unexpected,
+                        "failed to rename table as destination already exists",
+                    ))
+                } else if !src_res && dst_res {
                     Err(Error::new(
                         ErrorKind::Unexpected,
                         "failed to rename table as source does not exist",
                     ))
+                } else {
+                    Err(Error::new(ErrorKind::Unexpected, "failed to rename table"))
                 }
             }
-            Err(_) => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
+            _ => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
         }?;
 
         let query = format!(
@@ -683,18 +694,15 @@ impl Catalog for SqlCatalog {
         let src_table_exist = self.table_exists(src).await;
         let dst_table_exist = self.table_exists(dest).await;
 
-        match src_table_exist {
-            Ok(src_res) => match dst_table_exist {
-                Ok(dst_res) => {
-                    if !src_res && dst_res {
-                        Ok(())
-                    } else {
-                        Err(Error::new(ErrorKind::Unexpected, "failed to rename table"))
-                    }
+        match (src_table_exist, dst_table_exist) {
+            (Ok(src_res), Ok(dst_res)) => {
+                if !src_res && dst_res {
+                    Ok(())
+                } else {
+                    Err(Error::new(ErrorKind::Unexpected, "failed to rename table"))
                 }
-                Err(_) => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
-            },
-            Err(_) => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
+            }
+            _ => Err(Error::new(ErrorKind::Unexpected, "failed to rename table")),
         }
     }
 
@@ -806,8 +814,6 @@ pub mod tests {
             Namespace::with_properties(namespace.clone(), props.clone())
         );
 
-        //load table points to a /var location - check why
-
         let table = catalog.load_table(&identifier).await.unwrap();
 
         assert!(table.metadata().location().ends_with("/warehouse/table1"));