mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-05-24 08:50:40 +00:00
feat: support ALTER TABLE ... MODIFY COLUMN ... ... (#3796)
* feat: support `ALTER COLUMN xxx TYPE xxx` * fix: test `test_parse_alter_change_column_type` * style: code fmt * style: move to new test: `test_make_alter_column_type_region_request` * style: simplify the code * style: remove `v1::region::ChangeColumnType` * resolve conflicts * fix: test `test_make_alter_column_type_region_request` * style: simplify the code * rebase * rebase * rebase * fix: `ALTER COLUMN ... TYPE` -> `MODIFY COLUMN` * fix: `parser` -> `self.parser` * Apply suggestions from code review --------- Co-authored-by: Ruihang Xia <waynestxia@gmail.com> Co-authored-by: WenyXu <wenymedia@gmail.com>
This commit is contained in:
@@ -12,17 +12,18 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
use api::helper::ColumnDataTypeWrapper;
|
||||
use api::v1::add_column_location::LocationType;
|
||||
use api::v1::alter_expr::Kind;
|
||||
use api::v1::{
|
||||
column_def, AddColumnLocation as Location, AlterExpr, CreateTableExpr, DropColumns,
|
||||
RenameTable, SemanticType,
|
||||
column_def, AddColumnLocation as Location, AlterExpr, ChangeColumnTypes, CreateTableExpr,
|
||||
DropColumns, RenameTable, SemanticType,
|
||||
};
|
||||
use common_query::AddColumnLocation;
|
||||
use datatypes::schema::{ColumnSchema, RawSchema};
|
||||
use snafu::{ensure, OptionExt, ResultExt};
|
||||
use table::metadata::TableId;
|
||||
use table::requests::{AddColumnRequest, AlterKind, AlterTableRequest};
|
||||
use table::requests::{AddColumnRequest, AlterKind, AlterTableRequest, ChangeColumnTypeRequest};
|
||||
|
||||
use crate::error::{
|
||||
InvalidColumnDefSnafu, MissingFieldSnafu, MissingTimestampColumnSnafu, Result,
|
||||
@@ -64,13 +65,33 @@ pub fn alter_expr_to_request(table_id: TableId, expr: AlterExpr) -> Result<Alter
|
||||
columns: add_column_requests,
|
||||
}
|
||||
}
|
||||
Kind::ChangeColumnTypes(ChangeColumnTypes {
|
||||
change_column_types,
|
||||
}) => {
|
||||
let change_column_type_requests = change_column_types
|
||||
.into_iter()
|
||||
.map(|cct| {
|
||||
let target_type =
|
||||
ColumnDataTypeWrapper::new(cct.target_type(), cct.target_type_extension)
|
||||
.into();
|
||||
|
||||
Ok(ChangeColumnTypeRequest {
|
||||
column_name: cct.column_name,
|
||||
target_type,
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<_>>>()?;
|
||||
|
||||
AlterKind::ChangeColumnTypes {
|
||||
columns: change_column_type_requests,
|
||||
}
|
||||
}
|
||||
Kind::DropColumns(DropColumns { drop_columns }) => AlterKind::DropColumns {
|
||||
names: drop_columns.into_iter().map(|c| c.name).collect(),
|
||||
},
|
||||
Kind::RenameTable(RenameTable { new_table_name }) => {
|
||||
AlterKind::RenameTable { new_table_name }
|
||||
}
|
||||
Kind::ChangeColumnTypes(_) => unimplemented!(),
|
||||
};
|
||||
|
||||
let request = AlterTableRequest {
|
||||
@@ -138,7 +159,10 @@ fn parse_location(location: Option<Location>) -> Result<Option<AddColumnLocation
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use api::v1::{AddColumn, AddColumns, ColumnDataType, ColumnDef, DropColumn, SemanticType};
|
||||
use api::v1::{
|
||||
AddColumn, AddColumns, ChangeColumnType, ColumnDataType, ColumnDef, DropColumn,
|
||||
SemanticType,
|
||||
};
|
||||
use datatypes::prelude::ConcreteDataType;
|
||||
|
||||
use super::*;
|
||||
@@ -261,6 +285,40 @@ mod tests {
|
||||
assert_eq!(Some(AddColumnLocation::First), add_column.location);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_change_column_type_expr() {
|
||||
let expr = AlterExpr {
|
||||
catalog_name: "test_catalog".to_string(),
|
||||
schema_name: "test_schema".to_string(),
|
||||
table_name: "monitor".to_string(),
|
||||
|
||||
kind: Some(Kind::ChangeColumnTypes(ChangeColumnTypes {
|
||||
change_column_types: vec![ChangeColumnType {
|
||||
column_name: "mem_usage".to_string(),
|
||||
target_type: ColumnDataType::String as i32,
|
||||
target_type_extension: None,
|
||||
}],
|
||||
})),
|
||||
};
|
||||
|
||||
let alter_request = alter_expr_to_request(1, expr).unwrap();
|
||||
assert_eq!(alter_request.catalog_name, "test_catalog");
|
||||
assert_eq!(alter_request.schema_name, "test_schema");
|
||||
assert_eq!("monitor".to_string(), alter_request.table_name);
|
||||
|
||||
let mut change_column_types = match alter_request.alter_kind {
|
||||
AlterKind::ChangeColumnTypes { columns } => columns,
|
||||
_ => unreachable!(),
|
||||
};
|
||||
|
||||
let change_column_type = change_column_types.pop().unwrap();
|
||||
assert_eq!("mem_usage", change_column_type.column_name);
|
||||
assert_eq!(
|
||||
ConcreteDataType::string_datatype(),
|
||||
change_column_type.target_type
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_drop_column_expr() {
|
||||
let expr = AlterExpr {
|
||||
|
||||
@@ -91,6 +91,7 @@ fn create_proto_alter_kind(
|
||||
add_columns,
|
||||
})))
|
||||
}
|
||||
Kind::ChangeColumnTypes(x) => Ok(Some(alter_request::Kind::ChangeColumnTypes(x.clone()))),
|
||||
Kind::DropColumns(x) => {
|
||||
let drop_columns = x
|
||||
.drop_columns
|
||||
@@ -105,7 +106,6 @@ fn create_proto_alter_kind(
|
||||
})))
|
||||
}
|
||||
Kind::RenameTable(_) => Ok(None),
|
||||
Kind::ChangeColumnTypes(_) => unimplemented!(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,27 +119,27 @@ mod tests {
|
||||
use api::v1::region::region_request::Body;
|
||||
use api::v1::region::RegionColumnDef;
|
||||
use api::v1::{
|
||||
region, AddColumn, AddColumnLocation, AddColumns, AlterExpr, ColumnDataType,
|
||||
ColumnDef as PbColumnDef, SemanticType,
|
||||
region, AddColumn, AddColumnLocation, AddColumns, AlterExpr, ChangeColumnType,
|
||||
ChangeColumnTypes, ColumnDataType, ColumnDef as PbColumnDef, SemanticType,
|
||||
};
|
||||
use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME};
|
||||
use store_api::storage::RegionId;
|
||||
use store_api::storage::{RegionId, TableId};
|
||||
|
||||
use crate::ddl::alter_table::AlterTableProcedure;
|
||||
use crate::ddl::test_util::columns::TestColumnDefBuilder;
|
||||
use crate::ddl::test_util::create_table::{
|
||||
build_raw_table_info_from_expr, TestCreateTableExprBuilder,
|
||||
};
|
||||
use crate::ddl::DdlContext;
|
||||
use crate::key::table_route::TableRouteValue;
|
||||
use crate::peer::Peer;
|
||||
use crate::rpc::ddl::AlterTableTask;
|
||||
use crate::rpc::router::{Region, RegionRoute};
|
||||
use crate::test_util::{new_ddl_context, MockDatanodeManager};
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_make_alter_region_request() {
|
||||
let node_manager = Arc::new(MockDatanodeManager::new(()));
|
||||
let ddl_context = new_ddl_context(node_manager);
|
||||
async fn prepare_ddl_context() -> (DdlContext, u64, TableId, RegionId, String) {
|
||||
let datanode_manager = Arc::new(MockDatanodeManager::new(()));
|
||||
let ddl_context = new_ddl_context(datanode_manager);
|
||||
let cluster_id = 1;
|
||||
let table_id = 1024;
|
||||
let region_id = RegionId::new(table_id, 1);
|
||||
@@ -194,12 +194,25 @@ mod tests {
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
(
|
||||
ddl_context,
|
||||
cluster_id,
|
||||
table_id,
|
||||
region_id,
|
||||
table_name.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_make_alter_region_request() {
|
||||
let (ddl_context, cluster_id, table_id, region_id, table_name) =
|
||||
prepare_ddl_context().await;
|
||||
|
||||
let task = AlterTableTask {
|
||||
alter_table: AlterExpr {
|
||||
catalog_name: DEFAULT_CATALOG_NAME.to_string(),
|
||||
schema_name: DEFAULT_SCHEMA_NAME.to_string(),
|
||||
table_name: table_name.to_string(),
|
||||
table_name,
|
||||
kind: Some(Kind::AddColumns(AddColumns {
|
||||
add_columns: vec![AddColumn {
|
||||
column_def: Some(PbColumnDef {
|
||||
@@ -256,4 +269,48 @@ mod tests {
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_make_alter_column_type_region_request() {
|
||||
let (ddl_context, cluster_id, table_id, region_id, table_name) =
|
||||
prepare_ddl_context().await;
|
||||
|
||||
let task = AlterTableTask {
|
||||
alter_table: AlterExpr {
|
||||
catalog_name: DEFAULT_CATALOG_NAME.to_string(),
|
||||
schema_name: DEFAULT_SCHEMA_NAME.to_string(),
|
||||
table_name,
|
||||
kind: Some(Kind::ChangeColumnTypes(ChangeColumnTypes {
|
||||
change_column_types: vec![ChangeColumnType {
|
||||
column_name: "cpu".to_string(),
|
||||
target_type: ColumnDataType::String as i32,
|
||||
target_type_extension: None,
|
||||
}],
|
||||
})),
|
||||
},
|
||||
};
|
||||
|
||||
let mut procedure =
|
||||
AlterTableProcedure::new(cluster_id, table_id, task, ddl_context).unwrap();
|
||||
procedure.on_prepare().await.unwrap();
|
||||
let Some(Body::Alter(alter_region_request)) =
|
||||
procedure.make_alter_region_request(region_id).unwrap().body
|
||||
else {
|
||||
unreachable!()
|
||||
};
|
||||
assert_eq!(alter_region_request.region_id, region_id.as_u64());
|
||||
assert_eq!(alter_region_request.schema_version, 1);
|
||||
assert_eq!(
|
||||
alter_region_request.kind,
|
||||
Some(region::alter_request::Kind::ChangeColumnTypes(
|
||||
ChangeColumnTypes {
|
||||
change_column_types: vec![ChangeColumnType {
|
||||
column_name: "cpu".to_string(),
|
||||
target_type: ColumnDataType::String as i32,
|
||||
target_type_extension: None,
|
||||
}]
|
||||
}
|
||||
))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user