chore: fix test

GreptimeTeam · Jan 3, 2023 · f188958 · f188958
1 parent 42a5848
commit f188958
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 41 deletions.
diff --git a/src/datanode/src/sql/alter.rs b/src/datanode/src/sql/alter.rs
@@ -141,9 +141,21 @@ mod tests {
     async fn test_alter_to_request_with_renaming_table() {
         let handler = create_mock_sql_handler().await;
         let alter_table = parse_sql("ALTER TABLE test_table RENAME table_t;");
-        let err = handler
+        let req = handler
             .alter_to_request(alter_table, TableReference::bare("test_table"))
-            .unwrap_err();
-        assert_matches!(err, crate::error::Error::InvalidSql { .. });
+            .unwrap();
+        assert_eq!(req.catalog_name, Some("greptime".to_string()));
+        assert_eq!(req.schema_name, Some("public".to_string()));
+        assert_eq!(req.table_name, "test_table");
+
+        let alter_kind = req.alter_kind;
+        assert_matches!(alter_kind, AlterKind::RenameTable { .. });
+
+        match alter_kind {
+            AlterKind::RenameTable { new_table_name } => {
+                assert_eq!(new_table_name, "table_t");
+            }
+            _ => unreachable!(),
+        }
     }
 }
diff --git a/src/mito/src/table.rs b/src/mito/src/table.rs
@@ -170,11 +170,9 @@ impl<R: Region> Table for MitoTable<R> {
 
         let mut new_info = TableInfo::clone(&*table_info);
         // setup new table info
-        match req.alter_kind {
-            AlterKind::RenameTable {
-                new_table_name: ref new_name,
-            } => {
-                new_info.name = new_name.clone();
+        match &req.alter_kind {
+            AlterKind::RenameTable { new_table_name } => {
+                new_info.name = new_table_name.clone();
             }
             AlterKind::AddColumns { .. } | AlterKind::DropColumns { .. } => {
                 let table_meta = &table_info.meta;
@@ -208,34 +206,31 @@ impl<R: Region> Table for MitoTable<R> {
             })
             .map_err(BoxedError::new)
             .context(table_error::TableOperationSnafu)?;
-        if let AlterKind::RenameTable { .. } = &req.alter_kind {
-            // update memory metadata of the table
-            self.set_table_info(new_info);
-            return Ok(());
-        }
-
-        let alter_op = create_alter_operation(table_name, &req.alter_kind, &mut new_info.meta)?;
-        // TODO(yingwen): Error handling. Maybe the region need to provide a method to
-        // validate the request first.
-        let region = self.region();
-        let region_meta = region.in_memory_metadata();
-        let alter_req = AlterRequest {
-            operation: alter_op,
-            version: region_meta.version(),
-        };
-        // Alter the region.
-        logging::debug!(
-            "start altering region {} of table {}, with request {:?}",
-            region.name(),
-            table_name,
-            alter_req,
-        );
-        region
-            .alter(alter_req)
-            .await
-            .map_err(BoxedError::new)
-            .context(table_error::TableOperationSnafu)?;
 
+        if let Some(alter_op) =
+            create_alter_operation(table_name, &req.alter_kind, &mut new_info.meta)?
+        {
+            // TODO(yingwen): Error handling. Maybe the region need to provide a method to
+            // validate the request first.
+            let region = self.region();
+            let region_meta = region.in_memory_metadata();
+            let alter_req = AlterRequest {
+                operation: alter_op,
+                version: region_meta.version(),
+            };
+            // Alter the region.
+            logging::debug!(
+                "start altering region {} of table {}, with request {:?}",
+                region.name(),
+                table_name,
+                alter_req,
+            );
+            region
+                .alter(alter_req)
+                .await
+                .map_err(BoxedError::new)
+                .context(table_error::TableOperationSnafu)?;
+        }
         // Update in memory metadata of the table.
         self.set_table_info(new_info);
 
@@ -446,24 +441,24 @@ fn create_alter_operation(
     table_name: &str,
     alter_kind: &AlterKind,
     table_meta: &mut TableMeta,
-) -> TableResult<AlterOperation> {
+) -> TableResult<Option<AlterOperation>> {
     match alter_kind {
         AlterKind::AddColumns { columns } => {
             create_add_columns_operation(table_name, columns, table_meta)
         }
-        AlterKind::DropColumns { names } => Ok(AlterOperation::DropColumns {
+        AlterKind::DropColumns { names } => Ok(Some(AlterOperation::DropColumns {
             names: names.to_vec(),
-        }),
+        })),
         // No need to build alter operation when reaming tables.
-        AlterKind::RenameTable { .. } => unreachable!(),
+        AlterKind::RenameTable { .. } => Ok(None),
     }
 }
 
 fn create_add_columns_operation(
     table_name: &str,
     requests: &[AddColumnRequest],
     table_meta: &mut TableMeta,
-) -> TableResult<AlterOperation> {
+) -> TableResult<Option<AlterOperation>> {
     let columns = requests
         .iter()
         .map(|request| {
@@ -477,7 +472,7 @@ fn create_add_columns_operation(
         })
         .collect::<TableResult<Vec<_>>>()?;
 
-    Ok(AlterOperation::AddColumns { columns })
+    Ok(Some(AlterOperation::AddColumns { columns }))
 }
 
 #[cfg(test)]