From fc7e404aca1cb2738423003fa3d232ed69e22f0b Mon Sep 17 00:00:00 2001 From: Jinser Kafka Date: Wed, 18 Oct 2023 17:45:59 +0800 Subject: [PATCH] refactor: only support comment on `table` and `col` reduce the changes impact radius. --- proto/catalog.proto | 11 +- proto/ddl_service.proto | 8 +- proto/stream_plan.proto | 2 - src/connector/src/sink/catalog/desc.rs | 4 - src/connector/src/sink/catalog/mod.rs | 4 - src/frontend/src/catalog/catalog_service.rs | 12 +-- .../src/catalog/connection_catalog.rs | 2 - src/frontend/src/catalog/database_catalog.rs | 7 -- src/frontend/src/catalog/function_catalog.rs | 2 - src/frontend/src/catalog/index_catalog.rs | 4 - src/frontend/src/catalog/source_catalog.rs | 3 - .../src/catalog/system_catalog/mod.rs | 1 - .../rw_catalog/rw_description.rs | 100 +----------------- src/frontend/src/catalog/view_catalog.rs | 3 - .../src/expr/user_defined_function.rs | 1 - src/frontend/src/handler/comment.rs | 10 +- src/frontend/src/handler/create_function.rs | 1 - src/frontend/src/handler/create_index.rs | 1 - src/frontend/src/handler/create_source.rs | 1 - src/frontend/src/handler/create_table.rs | 1 - src/frontend/src/handler/create_view.rs | 1 - .../src/optimizer/plan_node/stream_sink.rs | 1 - src/frontend/src/test_utils.rs | 10 +- src/meta/src/controller/mod.rs | 2 - src/meta/src/manager/catalog/mod.rs | 12 +-- src/meta/src/model/catalog.rs | 1 - src/meta/src/rpc/ddl_controller.rs | 8 +- src/meta/src/rpc/service/ddl_service.rs | 11 +- src/rpc_client/src/meta_client.rs | 10 +- 29 files changed, 36 insertions(+), 198 deletions(-) diff --git a/proto/catalog.proto b/proto/catalog.proto index 4fd2c21cbfb73..7023d553f2d01 100644 --- a/proto/catalog.proto +++ b/proto/catalog.proto @@ -94,7 +94,6 @@ message Source { optional uint64 initialized_at_epoch = 15; optional uint64 created_at_epoch = 16; - optional string description = 17; // Per-source catalog version, used by schema change. uint64 version = 100; @@ -141,7 +140,6 @@ message Sink { string sink_from_name = 18; StreamJobStatus stream_job_status = 19; SinkFormatDesc format_desc = 20; - optional string description = 21; } message Connection { @@ -166,7 +164,6 @@ message Connection { PrivateLinkService private_link_service = 5; } uint32 owner = 6; - optional string description = 7; } message Index { @@ -185,7 +182,6 @@ message Index { optional uint64 initialized_at_epoch = 10; optional uint64 created_at_epoch = 11; StreamJobStatus stream_job_status = 12; - optional string description = 13; } message Function { @@ -208,8 +204,6 @@ message Function { message ScalarFunction {} message TableFunction {} message AggregateFunction {} - - optional string description = 14; } // See `TableCatalog` struct in frontend crate for more information. @@ -311,7 +305,6 @@ message View { repeated uint32 dependent_relations = 8; // User-specified column names. repeated plan_common.Field columns = 9; - optional string description = 10; } message Schema { @@ -319,18 +312,16 @@ message Schema { uint32 database_id = 2; string name = 3; uint32 owner = 4; - optional string description = 5; } message Database { uint32 id = 1; string name = 2; uint32 owner = 3; - optional string description = 4; } message Comment { uint32 table_id = 1; - uint32 column_index = 2; + optional uint32 column_index = 2; optional string description = 3; } diff --git a/proto/ddl_service.proto b/proto/ddl_service.proto index 29bd3e23260f8..05792623f408a 100644 --- a/proto/ddl_service.proto +++ b/proto/ddl_service.proto @@ -314,13 +314,13 @@ message GetTablesResponse { map tables = 1; } -message CreateCommentRequest { +message CommentOnRequest { uint32 table_id = 1; - uint32 column_index = 2; + optional uint32 column_index = 2; optional string comment = 3; } -message CreateCommentResponse { +message CommentOnResponse { common.Status status = 1; uint64 version = 2; } @@ -353,6 +353,6 @@ service DdlService { rpc CreateConnection(CreateConnectionRequest) returns (CreateConnectionResponse); rpc ListConnections(ListConnectionsRequest) returns (ListConnectionsResponse); rpc DropConnection(DropConnectionRequest) returns (DropConnectionResponse); - rpc CreateComment(CreateCommentRequest) returns (CreateCommentResponse); + rpc CommentOn(CommentOnRequest) returns (CommentOnResponse); rpc GetTables(GetTablesRequest) returns (GetTablesResponse); } diff --git a/proto/stream_plan.proto b/proto/stream_plan.proto index e33e674df7c04..a1026e0d36c1f 100644 --- a/proto/stream_plan.proto +++ b/proto/stream_plan.proto @@ -181,8 +181,6 @@ message SinkDesc { // it is the name of the sink itself. string sink_from_name = 12; catalog.SinkFormatDesc format_desc = 13; - - optional string description = 14; } enum SinkLogStoreType { diff --git a/src/connector/src/sink/catalog/desc.rs b/src/connector/src/sink/catalog/desc.rs index 6d1031ef6f7af..1fdd8b44959e3 100644 --- a/src/connector/src/sink/catalog/desc.rs +++ b/src/connector/src/sink/catalog/desc.rs @@ -64,8 +64,6 @@ pub struct SinkDesc { /// Name of the "table" field for Debezium. If the sink is from table or mv, /// it is the name of table/mv. Otherwise, it is the name of the sink. pub sink_from_name: String, - - pub description: Option, } impl SinkDesc { @@ -97,7 +95,6 @@ impl SinkDesc { initialized_at_epoch: None, db_name: self.db_name, sink_from_name: self.sink_from_name, - description: None, } } @@ -119,7 +116,6 @@ impl SinkDesc { format_desc: self.format_desc.as_ref().map(|f| f.to_proto()), db_name: self.db_name.clone(), sink_from_name: self.sink_from_name.clone(), - description: self.description.clone(), } } } diff --git a/src/connector/src/sink/catalog/mod.rs b/src/connector/src/sink/catalog/mod.rs index f81be598756a8..c18dd7d10a92c 100644 --- a/src/connector/src/sink/catalog/mod.rs +++ b/src/connector/src/sink/catalog/mod.rs @@ -293,8 +293,6 @@ pub struct SinkCatalog { /// Name for the table info for Debezium sink pub sink_from_name: String, - - pub description: Option, } impl SinkCatalog { @@ -332,7 +330,6 @@ impl SinkCatalog { db_name: self.db_name.clone(), sink_from_name: self.sink_from_name.clone(), stream_job_status: PbStreamJobStatus::Creating.into(), - description: self.description.clone(), } } @@ -421,7 +418,6 @@ impl From for SinkCatalog { initialized_at_epoch: pb.initialized_at_epoch.map(Epoch::from), db_name: pb.db_name, sink_from_name: pb.sink_from_name, - description: pb.description.clone(), } } } diff --git a/src/frontend/src/catalog/catalog_service.rs b/src/frontend/src/catalog/catalog_service.rs index 974824fe895ac..c1be43f260710 100644 --- a/src/frontend/src/catalog/catalog_service.rs +++ b/src/frontend/src/catalog/catalog_service.rs @@ -111,10 +111,10 @@ pub trait CatalogWriter: Send + Sync { connection: create_connection_request::Payload, ) -> Result<()>; - async fn create_comment( + async fn comment_on( &self, table_id: TableId, - column_index: u32, + column_index: Option, comment: Option, ) -> Result<()>; @@ -169,7 +169,6 @@ impl CatalogWriter for CatalogWriterImpl { name: db_name.to_string(), id: 0, owner, - description: None, }) .await?; self.wait_version(version).await @@ -188,7 +187,6 @@ impl CatalogWriter for CatalogWriterImpl { name: schema_name.to_string(), database_id: db_id, owner, - description: None, }) .await?; self.wait_version(version).await @@ -291,15 +289,15 @@ impl CatalogWriter for CatalogWriterImpl { self.wait_version(version).await } - async fn create_comment( + async fn comment_on( &self, TableId { table_id }: TableId, - column_index: u32, + column_index: Option, comment: Option, ) -> Result<()> { let version = self .meta_client - .create_comment(table_id, column_index, comment) + .comment_on(table_id, column_index, comment) .await?; self.wait_version(version).await } diff --git a/src/frontend/src/catalog/connection_catalog.rs b/src/frontend/src/catalog/connection_catalog.rs index 6b9cf7906e10a..7913d04379cd5 100644 --- a/src/frontend/src/catalog/connection_catalog.rs +++ b/src/frontend/src/catalog/connection_catalog.rs @@ -32,7 +32,6 @@ pub struct ConnectionCatalog { pub name: String, pub info: connection::Info, pub owner: UserId, - pub description: Option, } impl ConnectionCatalog { @@ -56,7 +55,6 @@ impl From<&PbConnection> for ConnectionCatalog { name: prost.name.clone(), info: prost.info.clone().unwrap(), owner: prost.owner, - description: prost.description.clone(), } } } diff --git a/src/frontend/src/catalog/database_catalog.rs b/src/frontend/src/catalog/database_catalog.rs index 81c510a6db8bd..76cfc0cd359c1 100644 --- a/src/frontend/src/catalog/database_catalog.rs +++ b/src/frontend/src/catalog/database_catalog.rs @@ -28,7 +28,6 @@ pub struct DatabaseCatalog { schema_by_name: HashMap, schema_name_by_id: HashMap, owner: u32, - description: Option, } impl DatabaseCatalog { @@ -66,7 +65,6 @@ impl DatabaseCatalog { database_id: self.id, name: schema.name(), owner: schema.owner(), - description: None, }) .collect_vec() } @@ -110,10 +108,6 @@ impl DatabaseCatalog { pub fn owner(&self) -> u32 { self.owner } - - pub fn description(&self) -> Option<&str> { - self.description.as_deref() - } } impl From<&PbDatabase> for DatabaseCatalog { fn from(db: &PbDatabase) -> Self { @@ -123,7 +117,6 @@ impl From<&PbDatabase> for DatabaseCatalog { schema_by_name: HashMap::new(), schema_name_by_id: HashMap::new(), owner: db.owner, - description: db.description.clone(), } } } diff --git a/src/frontend/src/catalog/function_catalog.rs b/src/frontend/src/catalog/function_catalog.rs index 5c5f54131b959..e56a36d85f8c2 100644 --- a/src/frontend/src/catalog/function_catalog.rs +++ b/src/frontend/src/catalog/function_catalog.rs @@ -31,7 +31,6 @@ pub struct FunctionCatalog { pub language: String, pub identifier: String, pub link: String, - pub description: Option, } #[derive(Clone, Display, PartialEq, Eq, Hash, Debug)] @@ -65,7 +64,6 @@ impl From<&PbFunction> for FunctionCatalog { language: prost.language.clone(), identifier: prost.identifier.clone(), link: prost.link.clone(), - description: prost.description.clone(), } } } diff --git a/src/frontend/src/catalog/index_catalog.rs b/src/frontend/src/catalog/index_catalog.rs index ed1bf2cf753da..ca4b4036332d3 100644 --- a/src/frontend/src/catalog/index_catalog.rs +++ b/src/frontend/src/catalog/index_catalog.rs @@ -63,8 +63,6 @@ pub struct IndexCatalog { pub created_at_epoch: Option, pub initialized_at_epoch: Option, - - pub description: Option, } impl IndexCatalog { @@ -126,7 +124,6 @@ impl IndexCatalog { original_columns, created_at_epoch: index_prost.created_at_epoch.map(Epoch::from), initialized_at_epoch: index_prost.initialized_at_epoch.map(Epoch::from), - description: index_prost.description.clone(), } } @@ -188,7 +185,6 @@ impl IndexCatalog { initialized_at_epoch: self.initialized_at_epoch.map(|e| e.0), created_at_epoch: self.created_at_epoch.map(|e| e.0), stream_job_status: PbStreamJobStatus::Creating.into(), - description: self.description.clone(), } } diff --git a/src/frontend/src/catalog/source_catalog.rs b/src/frontend/src/catalog/source_catalog.rs index 77eec6e01e339..ec35cfb7bde28 100644 --- a/src/frontend/src/catalog/source_catalog.rs +++ b/src/frontend/src/catalog/source_catalog.rs @@ -43,7 +43,6 @@ pub struct SourceCatalog { pub connection_id: Option, pub created_at_epoch: Option, pub initialized_at_epoch: Option, - pub description: Option, pub version: SourceVersionId, } @@ -73,7 +72,6 @@ impl SourceCatalog { optional_associated_table_id: self .associated_table_id .map(|id| OptionalAssociatedTableId::AssociatedTableId(id.table_id)), - description: self.description.clone(), version: self.version, } } @@ -129,7 +127,6 @@ impl From<&PbSource> for SourceCatalog { connection_id, created_at_epoch: prost.created_at_epoch.map(Epoch::from), initialized_at_epoch: prost.initialized_at_epoch.map(Epoch::from), - description: prost.description.clone(), version, } } diff --git a/src/frontend/src/catalog/system_catalog/mod.rs b/src/frontend/src/catalog/system_catalog/mod.rs index 0d68fda380da3..334f7e7f5cc7b 100644 --- a/src/frontend/src/catalog/system_catalog/mod.rs +++ b/src/frontend/src/catalog/system_catalog/mod.rs @@ -185,7 +185,6 @@ impl From<&BuiltinView> for ViewCatalog { sql: val.sql.to_string(), owner: DEFAULT_SUPER_USER_ID, properties: Default::default(), - description: None, } } } diff --git a/src/frontend/src/catalog/system_catalog/rw_catalog/rw_description.rs b/src/frontend/src/catalog/system_catalog/rw_catalog/rw_description.rs index 2303ab68dfae0..7d8bb7c6864dc 100644 --- a/src/frontend/src/catalog/system_catalog/rw_catalog/rw_description.rs +++ b/src/frontend/src/catalog/system_catalog/rw_catalog/rw_description.rs @@ -39,7 +39,7 @@ impl SysCatalogReaderImpl { OwnedRow::new(vec![ Some(ScalarImpl::Int32(table_id)), Some(ScalarImpl::Int32(catalog_id)), - Some(ScalarImpl::Int32(0)), + None, Some(ScalarImpl::Utf8(description)), ]) }; @@ -56,21 +56,9 @@ impl SysCatalogReaderImpl { let schemas = reader.iter_schemas_except_rw_catalog(&self.auth_context.database)?; let rw_catalog = reader.get_schema_by_name(&self.auth_context.database, "rw_catalog")?; - // XXX: is it shared object ?? - let database_desc = reader.iter_databases().map(|db| { - build_row( - db.id() as _, - rw_catalog - .get_system_table_by_name("rw_databases") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - db.description().unwrap_or_default().into(), - ) - }); - Ok(schemas .flat_map(|schema| { - let table_desc = schema.iter_table().flat_map(|table| { + schema.iter_table().flat_map(|table| { iter::once(build_row( table.id.table_id as _, rw_catalog @@ -83,7 +71,7 @@ impl SysCatalogReaderImpl { table .columns .iter() - .filter(|col| !col.is_hidden()) + // .filter(|col| !col.is_hidden()) .map(|col| { build_row_with_sub( table.id.table_id as _, @@ -101,88 +89,8 @@ impl SysCatalogReaderImpl { ) }), ) - }); - let mv_desc = schema.iter_mv().map(|mv| { - build_row( - mv.id.table_id as _, - rw_catalog - .get_system_table_by_name("rw_materialized_views") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - mv.description.as_deref().unwrap_or_default().into(), - ) - }); - let index_desc = schema.iter_index().map(|index| { - build_row( - index.id.index_id as _, - rw_catalog - .get_system_table_by_name("rw_indexes") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - index.description.as_deref().unwrap_or_default().into(), - ) - }); - let source_desc = schema.iter_source().map(|source| { - build_row( - source.id as _, - rw_catalog - .get_system_table_by_name("rw_sources") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - source.description.as_deref().unwrap_or_default().into(), - ) - }); - let sink_desc = schema.iter_sink().map(|sink| { - build_row( - sink.id.sink_id as _, - rw_catalog - .get_system_table_by_name("rw_sinks") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - sink.description.as_deref().unwrap_or_default().into(), - ) - }); - let view_desc = schema.iter_view().map(|view| { - build_row( - view.id as _, - rw_catalog - .get_system_table_by_name("rw_views") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - view.description.as_deref().unwrap_or_default().into(), - ) - }); - let function_desc = schema.iter_function().map(|function| { - build_row( - function.id.function_id() as _, - rw_catalog - .get_system_table_by_name("rw_functions") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - function.description.as_deref().unwrap_or_default().into(), - ) - }); - let connection_desc = schema.iter_connections().map(|connection| { - build_row( - connection.id as _, - rw_catalog - .get_system_table_by_name("rw_connetions") - .map(|st| st.id.table_id) - .unwrap_or_default() as _, - connection.description.as_deref().unwrap_or_default().into(), - ) - }); - - table_desc - .chain(mv_desc) - .chain(index_desc) - .chain(source_desc) - .chain(sink_desc) - .chain(view_desc) - .chain(function_desc) - .chain(connection_desc) + }) }) - .chain(database_desc) .collect()) } } diff --git a/src/frontend/src/catalog/view_catalog.rs b/src/frontend/src/catalog/view_catalog.rs index 1ceb62d12b4b0..b06d736d6d5a8 100644 --- a/src/frontend/src/catalog/view_catalog.rs +++ b/src/frontend/src/catalog/view_catalog.rs @@ -28,8 +28,6 @@ pub struct ViewCatalog { pub properties: WithOptions, pub sql: String, pub columns: Vec, - - pub description: Option, } impl From<&PbView> for ViewCatalog { @@ -41,7 +39,6 @@ impl From<&PbView> for ViewCatalog { properties: WithOptions::new(view.properties.clone()), sql: view.sql.clone(), columns: view.columns.iter().map(|f| f.into()).collect(), - description: view.description.clone(), } } } diff --git a/src/frontend/src/expr/user_defined_function.rs b/src/frontend/src/expr/user_defined_function.rs index e725cfe009ad3..1c9d06320ba15 100644 --- a/src/frontend/src/expr/user_defined_function.rs +++ b/src/frontend/src/expr/user_defined_function.rs @@ -56,7 +56,6 @@ impl UserDefinedFunction { language: udf.get_language().clone(), identifier: udf.get_identifier().clone(), link: udf.get_link().clone(), - description: None, }; Ok(Self { diff --git a/src/frontend/src/handler/comment.rs b/src/frontend/src/handler/comment.rs index 8c47b43069512..95716140e71d7 100644 --- a/src/frontend/src/handler/comment.rs +++ b/src/frontend/src/handler/comment.rs @@ -31,7 +31,6 @@ pub async fn handle_comment( let mut binder = Binder::new_for_ddl(&session); match object_type { CommentObject::Column => { - // TODO: wait to ask: How to bind `t.col` let [.., tab, col] = object_name.0.as_slice() else { return Err(ErrorCode::BindError(format!( "Invalid column: {}", @@ -47,22 +46,19 @@ pub async fn handle_comment( ( table.table_id, - column - .as_input_ref() - .map(|input_ref| input_ref.index + 1) // +1 since `_row_id` - .unwrap_or_default(), + column.as_input_ref().map(|input_ref| input_ref.index as _), ) } CommentObject::Table => { let table = binder.bind_table(None, &object_name.real_value(), None)?; - (table.table_id, 0) + (table.table_id, None) } } }; let catalog_writer = session.catalog_writer()?; catalog_writer - .create_comment(table_id, column_index as _, comment) + .comment_on(table_id, column_index, comment) .await?; Ok(PgResponse::empty_result(StatementType::COMMENT)) diff --git a/src/frontend/src/handler/create_function.rs b/src/frontend/src/handler/create_function.rs index a571498f2f4de..9d9db08204e49 100644 --- a/src/frontend/src/handler/create_function.rs +++ b/src/frontend/src/handler/create_function.rs @@ -171,7 +171,6 @@ pub async fn handle_create_function( identifier, link, owner: session.user_id(), - description: None, }; let catalog_writer = session.catalog_writer()?; diff --git a/src/frontend/src/handler/create_index.rs b/src/frontend/src/handler/create_index.rs index 193f167de4e5b..a5a002d3b3d79 100644 --- a/src/frontend/src/handler/create_index.rs +++ b/src/frontend/src/handler/create_index.rs @@ -243,7 +243,6 @@ pub(crate) fn gen_create_index_plan( initialized_at_epoch: None, created_at_epoch: None, stream_job_status: PbStreamJobStatus::Creating.into(), - description: None, }; let plan: PlanRef = materialize.into(); diff --git a/src/frontend/src/handler/create_source.rs b/src/frontend/src/handler/create_source.rs index 18547be0c3b06..29c38993a8f50 100644 --- a/src/frontend/src/handler/create_source.rs +++ b/src/frontend/src/handler/create_source.rs @@ -1156,7 +1156,6 @@ pub async fn handle_create_source( initialized_at_epoch: None, created_at_epoch: None, optional_associated_table_id: None, - description: None, version: INITIAL_SOURCE_VERSION_ID, }; diff --git a/src/frontend/src/handler/create_table.rs b/src/frontend/src/handler/create_table.rs index 00e98e98c63ab..6168c2500ce5b 100644 --- a/src/frontend/src/handler/create_table.rs +++ b/src/frontend/src/handler/create_table.rs @@ -686,7 +686,6 @@ fn gen_table_plan_inner( optional_associated_table_id: Some(OptionalAssociatedTableId::AssociatedTableId( TableId::placeholder().table_id, )), - description: None, version: INITIAL_SOURCE_VERSION_ID, }); diff --git a/src/frontend/src/handler/create_view.rs b/src/frontend/src/handler/create_view.rs index 9e6df885f4656..004c9a116f91a 100644 --- a/src/frontend/src/handler/create_view.rs +++ b/src/frontend/src/handler/create_view.rs @@ -103,7 +103,6 @@ pub async fn handle_create_view( .collect_vec(), sql: format!("{}", query), columns: columns.into_iter().map(|f| f.to_prost()).collect(), - description: None, }; let catalog_writer = session.catalog_writer()?; diff --git a/src/frontend/src/optimizer/plan_node/stream_sink.rs b/src/frontend/src/optimizer/plan_node/stream_sink.rs index 59e501b54338a..a51380d630331 100644 --- a/src/frontend/src/optimizer/plan_node/stream_sink.rs +++ b/src/frontend/src/optimizer/plan_node/stream_sink.rs @@ -183,7 +183,6 @@ impl StreamSink { properties: properties.into_inner(), sink_type, format_desc, - description: None, }; Ok((input, sink_desc)) } diff --git a/src/frontend/src/test_utils.rs b/src/frontend/src/test_utils.rs index 1567154a2d902..71921ddd4c594 100644 --- a/src/frontend/src/test_utils.rs +++ b/src/frontend/src/test_utils.rs @@ -204,7 +204,6 @@ impl CatalogWriter for MockCatalogWriter { name: db_name.to_string(), id: database_id, owner, - description: None, }); self.create_schema(database_id, DEFAULT_SCHEMA_NAME, owner) .await?; @@ -227,7 +226,6 @@ impl CatalogWriter for MockCatalogWriter { name: schema_name.to_string(), database_id: db_id, owner, - description: None, }); self.add_schema_id(id, db_id); Ok(()) @@ -320,10 +318,10 @@ impl CatalogWriter for MockCatalogWriter { unreachable!() } - async fn create_comment( + async fn comment_on( &self, _table_id: TableId, - _column_index: u32, + _column_index: Option, _comment: Option, ) -> Result<()> { unreachable!() @@ -508,28 +506,24 @@ impl MockCatalogWriter { id: 0, name: DEFAULT_DATABASE_NAME.to_string(), owner: DEFAULT_SUPER_USER_ID, - description: None, }); catalog.write().create_schema(&PbSchema { id: 1, name: DEFAULT_SCHEMA_NAME.to_string(), database_id: 0, owner: DEFAULT_SUPER_USER_ID, - description: None, }); catalog.write().create_schema(&PbSchema { id: 2, name: PG_CATALOG_SCHEMA_NAME.to_string(), database_id: 0, owner: DEFAULT_SUPER_USER_ID, - description: None, }); catalog.write().create_schema(&PbSchema { id: 3, name: RW_CATALOG_SCHEMA_NAME.to_string(), database_id: 0, owner: DEFAULT_SUPER_USER_ID, - description: None, }); let mut map: HashMap = HashMap::new(); map.insert(1_u32, 0_u32); diff --git a/src/meta/src/controller/mod.rs b/src/meta/src/controller/mod.rs index 102efe87d79fd..74f01497cc048 100644 --- a/src/meta/src/controller/mod.rs +++ b/src/meta/src/controller/mod.rs @@ -63,7 +63,6 @@ impl From> for PbDatabase { id: value.0.database_id as _, name: value.0.name, owner: value.1.owner_id as _, - description: None, } } } @@ -94,7 +93,6 @@ impl From> for PbSchema { name: value.0.name, database_id: value.0.database_id as _, owner: value.1.owner_id as _, - description: None, } } } diff --git a/src/meta/src/manager/catalog/mod.rs b/src/meta/src/manager/catalog/mod.rs index 569316e32510d..ac84f7e4afa57 100644 --- a/src/meta/src/manager/catalog/mod.rs +++ b/src/meta/src/manager/catalog/mod.rs @@ -218,7 +218,6 @@ impl CatalogManager { database_id: database.id, name: schema_name.to_string(), owner: database.owner, - description: None, }; schemas.insert(schema.id, schema.clone()); schemas_added.push(schema); @@ -2194,7 +2193,7 @@ impl CatalogManager { Ok(()) } - pub async fn create_comment(&self, comment: Comment) -> MetaResult { + pub async fn comment_on(&self, comment: Comment) -> MetaResult { let core = &mut *self.core.lock().await; let database_core = &mut core.database; database_core.ensure_table_view_or_source_id(&comment.table_id)?; @@ -2203,13 +2202,8 @@ impl CatalogManager { // TODO: dont unwrap let mut table = tables.get_mut(comment.table_id).unwrap(); - let col_idx = comment.column_index; - if col_idx > 0 { - let column = table - .columns - .iter_mut() - .find(|col| col.column_desc.clone().unwrap().column_id == col_idx as i32) - .unwrap(); + if let Some(col_idx) = comment.column_index { + let column = &mut table.columns[col_idx as usize]; column.column_desc.as_mut().unwrap().description = comment.description; } else { table.description = comment.description; diff --git a/src/meta/src/model/catalog.rs b/src/meta/src/model/catalog.rs index c18c3844da96a..add886ae862da 100644 --- a/src/meta/src/model/catalog.rs +++ b/src/meta/src/model/catalog.rs @@ -84,7 +84,6 @@ mod tests { id, name: format!("database_{}", id), owner: risingwave_common::catalog::DEFAULT_SUPER_USER_ID, - description: None, } } diff --git a/src/meta/src/rpc/ddl_controller.rs b/src/meta/src/rpc/ddl_controller.rs index c2bfb7f0fe794..cad6fc69b0aa8 100644 --- a/src/meta/src/rpc/ddl_controller.rs +++ b/src/meta/src/rpc/ddl_controller.rs @@ -100,7 +100,7 @@ pub enum DdlCommand { AlterSourceColumn(Source), CreateConnection(Connection), DropConnection(ConnectionId), - CreateComment(Comment), + CommentOn(Comment), } #[derive(Clone)] @@ -258,7 +258,7 @@ impl DdlController { ctrl.drop_connection(connection_id).await } DdlCommand::AlterSourceColumn(source) => ctrl.alter_source_column(source).await, - DdlCommand::CreateComment(comment) => ctrl.create_comment(comment).await, + DdlCommand::CommentOn(comment) => ctrl.comment_on(comment).await, } } .in_current_span(); @@ -1050,7 +1050,7 @@ impl DdlController { } } - async fn create_comment(&self, comment: Comment) -> MetaResult { - self.catalog_manager.create_comment(comment).await + async fn comment_on(&self, comment: Comment) -> MetaResult { + self.catalog_manager.comment_on(comment).await } } diff --git a/src/meta/src/rpc/service/ddl_service.rs b/src/meta/src/rpc/service/ddl_service.rs index b355c9f983a75..73e3189a0c65a 100644 --- a/src/meta/src/rpc/service/ddl_service.rs +++ b/src/meta/src/rpc/service/ddl_service.rs @@ -674,7 +674,6 @@ impl DdlService for DdlServiceImpl { name: req.name, owner: req.owner_id, info: Some(connection::Info::PrivateLinkService(private_link_svc)), - description: None, }; // save private link info to catalog @@ -718,22 +717,22 @@ impl DdlService for DdlServiceImpl { })) } - async fn create_comment( + async fn comment_on( &self, - request: Request, - ) -> Result, Status> { + request: Request, + ) -> Result, Status> { let req = request.into_inner(); let version = self .ddl_controller - .run_command(DdlCommand::CreateComment(Comment { + .run_command(DdlCommand::CommentOn(Comment { table_id: req.table_id, column_index: req.column_index, description: req.comment, })) .await?; - Ok(Response::new(CreateCommentResponse { + Ok(Response::new(CommentOnResponse { status: None, version, })) diff --git a/src/rpc_client/src/meta_client.rs b/src/rpc_client/src/meta_client.rs index eb9b37ca4fc3a..c5a48c16d22ff 100644 --- a/src/rpc_client/src/meta_client.rs +++ b/src/rpc_client/src/meta_client.rs @@ -406,18 +406,18 @@ impl MetaClient { Ok((resp.table_id.into(), resp.version)) } - pub async fn create_comment( + pub async fn comment_on( &self, table_id: u32, - column_index: u32, + column_index: Option, comment: Option, ) -> Result { - let request = CreateCommentRequest { + let request = CommentOnRequest { table_id, column_index, comment, }; - let resp = self.inner.create_comment(request).await?; + let resp = self.inner.comment_on(request).await?; Ok(resp.version) } @@ -1715,7 +1715,7 @@ macro_rules! for_all_meta_rpc { ,{ ddl_client, create_connection, CreateConnectionRequest, CreateConnectionResponse } ,{ ddl_client, list_connections, ListConnectionsRequest, ListConnectionsResponse } ,{ ddl_client, drop_connection, DropConnectionRequest, DropConnectionResponse } - ,{ ddl_client, create_comment, CreateCommentRequest, CreateCommentResponse } + ,{ ddl_client, comment_on, CommentOnRequest, CommentOnResponse } ,{ ddl_client, get_tables, GetTablesRequest, GetTablesResponse } ,{ hummock_client, unpin_version_before, UnpinVersionBeforeRequest, UnpinVersionBeforeResponse } ,{ hummock_client, get_current_version, GetCurrentVersionRequest, GetCurrentVersionResponse }