refactor: refactor error message return for drop table and set options

src/query/service/src/interpreters/interpreter_table_drop.rs

@@ -54,92 +54,96 @@ impl Interpreter for DropTableInterpreter {
         let catalog_name = self.plan.catalog.as_str();
         let db_name = self.plan.database.as_str();
         let tbl_name = self.plan.table.as_str();
-        let tbl = self
-            .ctx
-            .get_table(catalog_name, db_name, tbl_name)
-            .await
-            .ok();
+        let tbl = match self.ctx.get_table(catalog_name, db_name, tbl_name).await {
+            Ok(table) => table,
+            Err(error) => {
+                // UnknownTable Code
+                if error.code() == 1025 {
+                    if !self.plan.if_exists {
+                        return Err(ErrorCode::UnknownTable(format!(
+                            "Unknown table `{}`.`{}` in catalog '{}'",
+                            db_name, tbl_name, catalog_name
+                        )));
+                    }
+                    return Ok(PipelineBuildResult::create());
+                } else {
+                    return Err(error);
+                }
+            }
+        };
 
-        if tbl.is_none() && !self.plan.if_exists {
-            return Err(ErrorCode::UnknownTable(format!(
-                "Unknown table `{}`.`{}` in catalog '{}'",
-                db_name, tbl_name, catalog_name
+        let engine = tbl.get_table_info().engine();
+        if matches!(engine, VIEW_ENGINE | STREAM_ENGINE) {
+            return Err(ErrorCode::TableEngineNotSupported(format!(
+                "{}.{} engine is {} that doesn't support drop, use `DROP {} {}.{}` instead",
+                &self.plan.database,
+                &self.plan.table,
+                engine,
+                engine,
+                &self.plan.database,
+                &self.plan.table
             )));
         }
-        if let Some(tbl) = tbl {
-            let engine = tbl.get_table_info().engine();
-            if matches!(engine, VIEW_ENGINE | STREAM_ENGINE) {
-                return Err(ErrorCode::TableEngineNotSupported(format!(
-                    "{}.{} engine is {} that doesn't support drop, use `DROP {} {}.{}` instead",
-                    &self.plan.database,
-                    &self.plan.table,
-                    engine,
-                    engine,
-                    &self.plan.database,
-                    &self.plan.table
-                )));
-            }
-            let catalog = self.ctx.get_catalog(catalog_name).await?;
+        let catalog = self.ctx.get_catalog(catalog_name).await?;
 
-            // drop the ownership
-            let tenant = self.ctx.get_tenant();
-            let role_api = UserApiProvider::instance().get_role_api_client(&self.plan.tenant)?;
-            let db = catalog.get_database(&tenant, &self.plan.database).await?;
-            role_api
-                .drop_ownership(&GrantObjectByID::Table {
-                    catalog_name: self.plan.catalog.clone(),
-                    db_id: db.get_db_info().ident.db_id,
-                    table_id: tbl.get_table_info().ident.table_id,
-                })
-                .await?;
+        // drop the ownership
+        let tenant = self.ctx.get_tenant();
+        let role_api = UserApiProvider::instance().get_role_api_client(&self.plan.tenant)?;
+        let db = catalog.get_database(&tenant, &self.plan.database).await?;
+        role_api
+            .drop_ownership(&GrantObjectByID::Table {
+                catalog_name: self.plan.catalog.clone(),
+                db_id: db.get_db_info().ident.db_id,
+                table_id: tbl.get_table_info().ident.table_id,
+            })
+            .await?;
 
-            // Although even if data is in READ_ONLY mode,
-            // as a catalog object, the table itself is allowed to be dropped (and undropped later),
-            // `drop table ALL` is NOT allowed, which implies that the table data need to be truncated.
-            if self.plan.all {
-                // check mutability, if the table is read only, we cannot truncate the data
-                tbl.check_mutable().map_err(|e| {
+        // Although even if data is in READ_ONLY mode,
+        // as a catalog object, the table itself is allowed to be dropped (and undropped later),
+        // `drop table ALL` is NOT allowed, which implies that the table data need to be truncated.
+        if self.plan.all {
+            // check mutability, if the table is read only, we cannot truncate the data
+            tbl.check_mutable().map_err(|e| {
                     e.add_message(" drop table ALL is not allowed for read only table, please consider remove the option ALL")
                 })?
-            }
+        }
 
-            // actually drop table
-            let resp = catalog
-                .drop_table_by_id(DropTableByIdReq {
-                    if_exists: self.plan.if_exists,
-                    tenant,
-                    table_name: tbl_name.to_string(),
-                    tb_id: tbl.get_table_info().ident.table_id,
-                    db_id: db.get_db_info().ident.db_id,
-                })
-                .await?;
+        // actually drop table
+        let resp = catalog
+            .drop_table_by_id(DropTableByIdReq {
+                if_exists: self.plan.if_exists,
+                tenant,
+                table_name: tbl_name.to_string(),
+                tb_id: tbl.get_table_info().ident.table_id,
+                db_id: db.get_db_info().ident.db_id,
+            })
+            .await?;
 
-            // if `plan.all`, truncate, then purge the historical data
-            if self.plan.all {
-                // the above `catalog.drop_table` operation changed the table meta version,
-                // thus if we do not refresh the table instance, `truncate` will fail
-                let latest = tbl.as_ref().refresh(self.ctx.as_ref()).await?;
-                let maybe_fuse_table = FuseTable::try_from_table(latest.as_ref());
-                // if target table if of type FuseTable, purge its historical data
-                // otherwise, plain truncate
-                if let Ok(fuse_table) = maybe_fuse_table {
-                    let purge = true;
-                    fuse_table.do_truncate(self.ctx.clone(), purge).await?
-                } else {
-                    latest.truncate(self.ctx.clone()).await?
-                }
+        // if `plan.all`, truncate, then purge the historical data
+        if self.plan.all {
+            // the above `catalog.drop_table` operation changed the table meta version,
+            // thus if we do not refresh the table instance, `truncate` will fail
+            let latest = tbl.as_ref().refresh(self.ctx.as_ref()).await?;
+            let maybe_fuse_table = FuseTable::try_from_table(latest.as_ref());
+            // if target table if of type FuseTable, purge its historical data
+            // otherwise, plain truncate
+            if let Ok(fuse_table) = maybe_fuse_table {
+                let purge = true;
+                fuse_table.do_truncate(self.ctx.clone(), purge).await?
+            } else {
+                latest.truncate(self.ctx.clone()).await?
             }
+        }
 
-            // update share spec if needed
-            if let Some((spec_vec, share_table_info)) = resp.spec_vec {
-                save_share_spec(
-                    &self.ctx.get_tenant(),
-                    self.ctx.get_data_operator()?.operator(),
-                    Some(spec_vec),
-                    Some(share_table_info),
-                )
-                .await?;
-            }
+        // update share spec if needed
+        if let Some((spec_vec, share_table_info)) = resp.spec_vec {
+            save_share_spec(
+                &self.ctx.get_tenant(),
+                self.ctx.get_data_operator()?.operator(),
+                Some(spec_vec),
+                Some(share_table_info),
+            )
+            .await?;
         }
 
         Ok(PipelineBuildResult::create())

src/query/service/src/interpreters/interpreter_table_set_options.rs

@@ -89,25 +89,30 @@ impl Interpreter for SetOptionsInterpreter {
         }
         let catalog = self.ctx.get_catalog(self.plan.catalog.as_str()).await?;
         let database = self.plan.database.as_str();
-        let table = self.plan.table.as_str();
-        let tbl = catalog
-            .get_table(self.ctx.get_tenant().as_str(), database, table)
+        let table_name = self.plan.table.as_str();
+        let table = match catalog
+            .get_table(self.ctx.get_tenant().as_str(), database, table_name)
             .await
-            .ok();
-
-        let table = if let Some(table) = &tbl {
-            // check mutability
-            table.check_mutable()?;
-            table
-        } else {
-            return Err(ErrorCode::UnknownTable(format!(
-                "Unknown table `{}`.`{}` in catalog '{}'",
-                database,
-                self.plan.table.as_str(),
-                &catalog.name()
-            )));
+        {
+            Ok(table) => table,
+            Err(error) => {
+                // UnknownTable Code
+                if error.code() == 1025 {
+                    return Err(ErrorCode::UnknownTable(format!(
+                        "Unknown table `{}`.`{}` in catalog '{}'",
+                        database,
+                        self.plan.table.as_str(),
+                        &catalog.name()
+                    )));
+                } else {
+                    return Err(error);
+                }
+            }
         };
 
+        // check mutability
+        table.check_mutable()?;
+
         // check bloom_index_columns.
         is_valid_bloom_index_columns(&self.plan.set_options, table.schema())?;