From b3b43fe1c3d6a67bcc57f2f8a285b706a99e5786 Mon Sep 17 00:00:00 2001 From: Niwaka <61189782+NiwakaDev@users.noreply.github.com> Date: Tue, 22 Aug 2023 12:53:56 +0900 Subject: [PATCH] fix: table options can't be found in distributed mode (#2209) * fix: table options can't be found in distributed mode * refactor: use iterator for regions_numbers * chore: remove TODO --- src/common/meta/src/key.rs | 33 ++++++--- src/frontend/src/instance/distributed.rs | 1 + src/meta-srv/src/procedure/create_table.rs | 2 - tests-integration/src/tests/instance_test.rs | 69 ++++++++++++------- .../cases/distributed/show/show_create.result | 8 ++- 5 files changed, 73 insertions(+), 40 deletions(-) diff --git a/src/common/meta/src/key.rs b/src/common/meta/src/key.rs index 525274a40fbb..c1d1b54ee3c6 100644 --- a/src/common/meta/src/key.rs +++ b/src/common/meta/src/key.rs @@ -215,9 +215,14 @@ impl TableMetadataManager { /// The caller MUST ensure it has the exclusive access to `TableNameKey`. pub async fn create_table_metadata( &self, - table_info: RawTableInfo, + mut table_info: RawTableInfo, region_routes: Vec, ) -> Result<()> { + let region_numbers = region_routes + .iter() + .map(|region| region.region.id.region_number()) + .collect::>(); + table_info.meta.region_numbers = region_numbers; let table_id = table_info.ident.table_id; // Creates table name. @@ -524,7 +529,7 @@ mod tests { assert_eq!(removed, to_removed_key(key)); } - fn new_test_table_info() -> TableInfo { + fn new_test_table_info(region_numbers: impl Iterator) -> TableInfo { let column_schemas = vec![ ColumnSchema::new("col1", ConcreteDataType::int32_datatype(), true), ColumnSchema::new( @@ -546,6 +551,7 @@ mod tests { .primary_key_indices(vec![0]) .engine("engine") .next_column_id(3) + .region_numbers(region_numbers.collect::>()) .build() .unwrap(); TableInfoBuilder::default() @@ -578,9 +584,10 @@ mod tests { async fn test_create_table_metadata() { let mem_kv = Arc::new(MemoryKvBackend::default()); let table_metadata_manager = TableMetadataManager::new(mem_kv); - let table_info: RawTableInfo = new_test_table_info().into(); let region_route = new_test_region_route(); let region_routes = vec![region_route.clone()]; + let table_info: RawTableInfo = + new_test_table_info(region_routes.iter().map(|r| r.region.id.region_number())).into(); // creates metadata. table_metadata_manager .create_table_metadata(table_info.clone(), region_routes.clone()) @@ -612,11 +619,12 @@ mod tests { async fn test_delete_table_metadata() { let mem_kv = Arc::new(MemoryKvBackend::default()); let table_metadata_manager = TableMetadataManager::new(mem_kv); - let table_info: RawTableInfo = new_test_table_info().into(); - let table_id = table_info.ident.table_id; let region_route = new_test_region_route(); - let datanode_id = 2; let region_routes = vec![region_route.clone()]; + let table_info: RawTableInfo = + new_test_table_info(region_routes.iter().map(|r| r.region.id.region_number())).into(); + let table_id = table_info.ident.table_id; + let datanode_id = 2; let table_route_value = TableRouteValue::new(region_routes.clone()); // creates metadata. @@ -682,10 +690,11 @@ mod tests { async fn test_rename_table() { let mem_kv = Arc::new(MemoryKvBackend::default()); let table_metadata_manager = TableMetadataManager::new(mem_kv); - let table_info: RawTableInfo = new_test_table_info().into(); - let table_id = table_info.ident.table_id; let region_route = new_test_region_route(); let region_routes = vec![region_route.clone()]; + let table_info: RawTableInfo = + new_test_table_info(region_routes.iter().map(|r| r.region.id.region_number())).into(); + let table_id = table_info.ident.table_id; // creates metadata. table_metadata_manager .create_table_metadata(table_info.clone(), region_routes.clone()) @@ -746,10 +755,11 @@ mod tests { async fn test_update_table_info() { let mem_kv = Arc::new(MemoryKvBackend::default()); let table_metadata_manager = TableMetadataManager::new(mem_kv); - let table_info: RawTableInfo = new_test_table_info().into(); - let table_id = table_info.ident.table_id; let region_route = new_test_region_route(); let region_routes = vec![region_route.clone()]; + let table_info: RawTableInfo = + new_test_table_info(region_routes.iter().map(|r| r.region.id.region_number())).into(); + let table_id = table_info.ident.table_id; // creates metadata. table_metadata_manager .create_table_metadata(table_info.clone(), region_routes.clone()) @@ -811,9 +821,10 @@ mod tests { async fn test_update_table_route() { let mem_kv = Arc::new(MemoryKvBackend::default()); let table_metadata_manager = TableMetadataManager::new(mem_kv); - let table_info: RawTableInfo = new_test_table_info().into(); let region_route = new_test_region_route(); let region_routes = vec![region_route.clone()]; + let table_info: RawTableInfo = + new_test_table_info(region_routes.iter().map(|r| r.region.id.region_number())).into(); let table_id = table_info.ident.table_id; let current_table_route_value = TableRouteValue::new(region_routes.clone()); // creates metadata. diff --git a/src/frontend/src/instance/distributed.rs b/src/frontend/src/instance/distributed.rs index bb59dec2ae62..c2ccbb3cb0eb 100644 --- a/src/frontend/src/instance/distributed.rs +++ b/src/frontend/src/instance/distributed.rs @@ -121,6 +121,7 @@ impl DistInstance { info!("Successfully created distributed table '{table_name}' with table id {table_id}"); table_info.ident.table_id = table_id; + let table_info = Arc::new(table_info.try_into().context(error::CreateTableInfoSnafu)?); create_table.table_id = Some(api::v1::TableId { id: table_id }); diff --git a/src/meta-srv/src/procedure/create_table.rs b/src/meta-srv/src/procedure/create_table.rs index af512ae62538..01d1813728ea 100644 --- a/src/meta-srv/src/procedure/create_table.rs +++ b/src/meta-srv/src/procedure/create_table.rs @@ -110,12 +110,10 @@ impl CreateTableProcedure { ); let table_id = self.table_info().ident.table_id as TableId; - let manager = &self.context.table_metadata_manager; let raw_table_info = self.table_info().clone(); let region_routes = self.region_routes().clone(); - manager .create_table_metadata(raw_table_info, region_routes) .await diff --git a/tests-integration/src/tests/instance_test.rs b/tests-integration/src/tests/instance_test.rs index d2c2348333e4..364da5c1af8f 100644 --- a/tests-integration/src/tests/instance_test.rs +++ b/tests-integration/src/tests/instance_test.rs @@ -80,38 +80,57 @@ async fn test_create_database_and_insert_query(instance: Arc) #[apply(both_instances_cases)] async fn test_show_create_table(instance: Arc) { let frontend = instance.frontend(); - - let output = execute_sql( - &frontend, + let sql = if instance.is_distributed_mode() { r#"create table demo( - host STRING, - cpu DOUBLE, - memory DOUBLE, - ts bigint, - TIME INDEX(ts) -)"#, - ) - .await; + host STRING, + cpu DOUBLE, + memory DOUBLE, + ts bigint, + TIME INDEX(ts) +) +PARTITION BY RANGE COLUMNS (ts) ( + PARTITION r0 VALUES LESS THAN (1), + PARTITION r1 VALUES LESS THAN (10), + PARTITION r2 VALUES LESS THAN (100), + PARTITION r3 VALUES LESS THAN (MAXVALUE), +)"# + } else { + r#"create table demo( + host STRING, + cpu DOUBLE, + memory DOUBLE, + ts bigint, + TIME INDEX(ts) +)"# + }; + let output = execute_sql(&frontend, sql).await; assert!(matches!(output, Output::AffectedRows(0))); let output = execute_sql(&frontend, "show create table demo").await; let expected = if instance.is_distributed_mode() { "\ -+-------+-----------------------------------+ -| Table | Create Table | -+-------+-----------------------------------+ -| demo | CREATE TABLE IF NOT EXISTS demo ( | -| | host STRING NULL, | -| | cpu DOUBLE NULL, | -| | memory DOUBLE NULL, | -| | ts BIGINT NOT NULL, | -| | TIME INDEX (ts) | -| | ) | -| | | -| | ENGINE=mito | -| | | -+-------+-----------------------------------+" ++-------+----------------------------------------------------------+ +| Table | Create Table | ++-------+----------------------------------------------------------+ +| demo | CREATE TABLE IF NOT EXISTS demo ( | +| | host STRING NULL, | +| | cpu DOUBLE NULL, | +| | memory DOUBLE NULL, | +| | ts BIGINT NOT NULL, | +| | TIME INDEX (ts) | +| | ) | +| | PARTITION BY RANGE COLUMNS (ts) ( | +| | PARTITION r0 VALUES LESS THAN (1), | +| | PARTITION r1 VALUES LESS THAN (10), | +| | PARTITION r2 VALUES LESS THAN (100), | +| | PARTITION r3 VALUES LESS THAN (MAXVALUE) | +| | ) | +| | ENGINE=mito | +| | WITH( | +| | regions = 4 | +| | ) | ++-------+----------------------------------------------------------+" } else { "\ +-------+-----------------------------------+ diff --git a/tests/cases/distributed/show/show_create.result b/tests/cases/distributed/show/show_create.result index 93f73a3a203f..42e76749453f 100644 --- a/tests/cases/distributed/show/show_create.result +++ b/tests/cases/distributed/show/show_create.result @@ -38,7 +38,9 @@ SHOW CREATE TABLE system_metrics; | | PARTITION r2 VALUES LESS THAN (MAXVALUE) | | | ) | | | ENGINE=mito | -| | | +| | WITH( | +| | regions = 3 | +| | ) | +----------------+----------------------------------------------------------+ DROP TABLE system_metrics; @@ -62,7 +64,9 @@ show create table table_without_partition; | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | regions = 1 | +| | ) | +-------------------------+---------------------------------------------------------+ drop table table_without_partition;