From 7cc188209222f1cd8b0ae716254b73bc617ab40b Mon Sep 17 00:00:00 2001 From: CookiePieWw Date: Wed, 6 Nov 2024 19:01:51 +0800 Subject: [PATCH] feat: alter column fulltext option Co-Authored-By: irenjj --- src/datatypes/src/schema/column_schema.rs | 8 +++ src/store-api/src/metadata.rs | 70 ++++++++++++++++-- src/store-api/src/region_request.rs | 4 +- src/table/src/error.rs | 13 ++++ src/table/src/metadata.rs | 86 +++++++++++++++++++++-- 5 files changed, 168 insertions(+), 13 deletions(-) diff --git a/src/datatypes/src/schema/column_schema.rs b/src/datatypes/src/schema/column_schema.rs index dabaf97d904b..57e582e973e3 100644 --- a/src/datatypes/src/schema/column_schema.rs +++ b/src/datatypes/src/schema/column_schema.rs @@ -270,6 +270,14 @@ impl ColumnSchema { ); Ok(self) } + + pub fn set_fulltext_options(&mut self, options: &FulltextOptions) -> Result<()> { + self.metadata.insert( + FULLTEXT_KEY.to_string(), + serde_json::to_string(options).context(error::SerializeSnafu)?, + ); + Ok(()) + } } impl TryFrom<&Field> for ColumnSchema { diff --git a/src/store-api/src/metadata.rs b/src/store-api/src/metadata.rs index 8c1fcabe0dcd..ee538f658888 100644 --- a/src/store-api/src/metadata.rs +++ b/src/store-api/src/metadata.rs @@ -526,7 +526,7 @@ impl RegionMetadataBuilder { AlterKind::ChangeColumnFulltext { column_name, options, - } => self.change_column_fulltext_options(column_name, options), + } => self.change_column_fulltext_options(column_name, options)?, AlterKind::ChangeRegionOptions { options: _ } => { // nothing to be done with RegionMetadata } @@ -630,11 +630,56 @@ impl RegionMetadataBuilder { } } - fn change_column_fulltext_options(&mut self, column_name: String, _options: FulltextOptions) { + fn change_column_fulltext_options( + &mut self, + column_name: String, + options: FulltextOptions, + ) -> Result<()> { for column_meta in self.column_metadatas.iter_mut() { - if column_meta.column_schema.name == column_name {} + if column_meta.column_schema.name == column_name { + ensure!( + column_meta.column_schema.data_type.is_string(), + SetFulltextOptionsSnafu { + column_name: column_name.to_string(), + reason: format!( + "column {} is not a string type, but {:?}", + column_name, column_meta.column_schema.data_type + ), + } + ); + + let current_fulltext_options = + column_meta.column_schema.fulltext_options().map_err(|e| { + SetFulltextOptionsSnafu { + column_name: column_name.clone(), + reason: e.to_string(), + } + .build() + })?; + + // Don't allow to change fulltext options if it is already enabled. + if !(current_fulltext_options.is_some_and(|o| o.enable) && options.enable) { + column_meta + .column_schema + .set_fulltext_options(&options) + .map_err(|e| { + SetFulltextOptionsSnafu { + column_name: column_name.clone(), + reason: e.to_string(), + } + .build() + })?; + } else { + return SetFulltextOptionsSnafu { + column_name, + reason: "fulltext options already enabled".to_string(), + } + .fail(); + } + break; + } } - todo!() + Ok(()) } } @@ -761,9 +806,20 @@ pub enum MetadataError { location: Location, }, - #[snafu(display("Fail to parse fulltext analyzer proto"))] - InvalidFulltextAnalyzerProto { - source: api::error::Error, + #[snafu(display("Invalid change fulltext option request"))] + InvalidChangeFulltextOptionRequest { + #[snafu(source)] + error: api::error::Error, + }, + + #[snafu(display( + "Failed to set fulltext options for column {}, reason: {}", + column_name, + reason + ))] + SetFulltextOptions { + column_name: String, + reason: String, #[snafu(implicit)] location: Location, }, diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index 1735a5e1d6ac..bb788b3d2398 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -34,7 +34,7 @@ use strum::IntoStaticStr; use crate::logstore::entry; use crate::metadata::{ - ColumnMetadata, InvalidFulltextAnalyzerProtoSnafu, InvalidRawRegionRequestSnafu, + ColumnMetadata, InvalidChangeFulltextOptionRequestSnafu, InvalidRawRegionRequestSnafu, InvalidRegionOptionChangeRequestSnafu, InvalidRegionRequestSnafu, MetadataError, RegionMetadata, Result, }; @@ -538,7 +538,7 @@ impl TryFrom for AlterKind { options: FulltextOptions { enable: x.enable, analyzer: try_as_fulltext_option(x.analyzer) - .context(InvalidFulltextAnalyzerProtoSnafu)?, + .context(InvalidChangeFulltextOptionRequestSnafu)?, case_sensitive: x.case_sensitive, }, }, diff --git a/src/table/src/error.rs b/src/table/src/error.rs index e6ea1e19325f..ab89028b82ae 100644 --- a/src/table/src/error.rs +++ b/src/table/src/error.rs @@ -140,6 +140,18 @@ pub enum Error { #[snafu(display("Table options value is not valid, key: `{}`, value: `{}`", key, value))] InvalidTableOptionValue { key: String, value: String }, + + #[snafu(display( + "Failed to set fulltext options for column {}, reason: {}", + column_name, + reason + ))] + SetFulltextOptions { + column_name: String, + reason: String, + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -156,6 +168,7 @@ impl ErrorExt for Error { Error::ColumnExists { .. } => StatusCode::TableColumnExists, Error::SchemaBuild { source, .. } => source.status_code(), Error::TableOperation { source } => source.status_code(), + Error::SetFulltextOptions { .. } => StatusCode::InvalidArguments, Error::ColumnNotExists { .. } => StatusCode::TableColumnNotFound, Error::Unsupported { .. } => StatusCode::Unsupported, Error::ParseTableOption { .. } => StatusCode::InvalidArguments, diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs index 79a65d2a5c40..3caa37cfe5cd 100644 --- a/src/table/src/metadata.rs +++ b/src/table/src/metadata.rs @@ -216,7 +216,7 @@ impl TableMeta { AlterKind::ChangeColumnFulltext { column_name, options, - } => self.change_column_fulltext_options(column_name, options), + } => self.change_column_fulltext_options(table_name, column_name, options), } } @@ -248,10 +248,88 @@ impl TableMeta { /// Creates a [TableMetaBuilder] with modified column fulltext options. fn change_column_fulltext_options( &self, - _column_name: &str, - _options: &FulltextOptions, + table_name: &str, + column_name: &str, + options: &FulltextOptions, ) -> Result { - todo!() + let table_schema = &self.schema; + let mut meta_builder = self.new_meta_builder(); + + let column = &table_schema + .column_schema_by_name(column_name) + .with_context(|| error::ColumnNotExistsSnafu { + column_name, + table_name, + })?; + + ensure!( + column.data_type.is_string(), + error::SetFulltextOptionsSnafu { + column_name: column_name.to_string(), + reason: format!( + "column {} is not a string type, but {:?}", + column_name, column.data_type + ), + } + ); + + let current_fulltext_options = column.fulltext_options().map_err(|e| { + error::SetFulltextOptionsSnafu { + column_name: column_name.to_string(), + reason: e.to_string(), + } + .build() + })?; + + ensure!( + !(current_fulltext_options.is_some_and(|o| o.enable) && options.enable), + error::SetFulltextOptionsSnafu { + column_name: column_name.to_string(), + reason: "fulltext options already enabled".to_string(), + } + ); + + let mut columns = vec![]; + for column_schema in table_schema.column_schemas() { + if column_schema.name == column_name { + let mut new_column_schema = column_schema.clone(); + new_column_schema + .set_fulltext_options(options) + .map_err(|e| { + error::SetFulltextOptionsSnafu { + column_name: column_name.to_string(), + reason: e.to_string(), + } + .build() + })?; + columns.push(new_column_schema); + } else { + columns.push(column_schema.clone()); + } + } + + // TODO(CookiePieWw): This part for all alter table operations is similar. We can refactor it. + let mut builder = SchemaBuilder::try_from_columns(columns) + .with_context(|_| error::SchemaBuildSnafu { + msg: format!("Failed to convert column schemas into schema for table {table_name}"), + })? + .version(table_schema.version() + 1); + + for (k, v) in table_schema.metadata().iter() { + builder = builder.add_metadata(k, v); + } + + let new_schema = builder.build().with_context(|_| error::SchemaBuildSnafu { + msg: format!( + "Table {table_name} cannot change fulltext options for column {column_name}", + ), + })?; + + let _ = meta_builder + .schema(Arc::new(new_schema)) + .primary_key_indices(self.primary_key_indices.clone()); + + Ok(meta_builder) } /// Allocate a new column for the table.