diff --git a/Cargo.lock b/Cargo.lock index 7356a706f8..507bef7d5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5158,7 +5158,7 @@ dependencies = [ [[package]] name = "greptime-proto" version = "0.1.0" -source = "git+https://github.com/GreptimeTeam/greptime-proto.git?rev=17f14bc2791a336196ac0f4d2df9ee2972233ab8" +source = "git+https://github.com/GreptimeTeam/greptime-proto.git?rev=0c4720be4c9ab037e41df07da1fc237b94f7aea4#0c4720be4c9ab037e41df07da1fc237b94f7aea4" dependencies = [ "prost 0.13.5", "serde", diff --git a/Cargo.toml b/Cargo.toml index e773eb2315..5f93c30b35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -134,7 +134,7 @@ etcd-client = "0.14" fst = "0.4.7" futures = "0.3" futures-util = "0.3" -greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "17f14bc2791a336196ac0f4d2df9ee2972233ab8" } +greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "0c4720be4c9ab037e41df07da1fc237b94f7aea4" } hex = "0.4" http = "1" humantime = "2.1" diff --git a/src/common/grpc-expr/src/alter.rs b/src/common/grpc-expr/src/alter.rs index ee80d9551e..ba3791432d 100644 --- a/src/common/grpc-expr/src/alter.rs +++ b/src/common/grpc-expr/src/alter.rs @@ -29,8 +29,8 @@ use snafu::{ensure, OptionExt, ResultExt}; use store_api::region_request::{SetRegionOption, UnsetRegionOption}; use table::metadata::TableId; use table::requests::{ - AddColumnRequest, AlterKind, AlterTableRequest, ModifyColumnTypeRequest, SetIndexOptions, - UnsetIndexOptions, + AddColumnRequest, AlterKind, AlterTableRequest, ModifyColumnTypeRequest, SetIndexOption, + UnsetIndexOption, }; use crate::error::{ @@ -43,6 +43,59 @@ use crate::error::{ const LOCATION_TYPE_FIRST: i32 = LocationType::First as i32; const LOCATION_TYPE_AFTER: i32 = LocationType::After as i32; +fn set_index_option_from_proto(set_index: api::v1::SetIndex) -> Result { + let options = set_index.options.context(MissingAlterIndexOptionSnafu)?; + Ok(match options { + api::v1::set_index::Options::Fulltext(f) => SetIndexOption::Fulltext { + column_name: f.column_name.clone(), + options: FulltextOptions::new( + f.enable, + as_fulltext_option_analyzer( + Analyzer::try_from(f.analyzer).context(InvalidSetFulltextOptionRequestSnafu)?, + ), + f.case_sensitive, + as_fulltext_option_backend( + PbFulltextBackend::try_from(f.backend) + .context(InvalidSetFulltextOptionRequestSnafu)?, + ), + f.granularity as u32, + f.false_positive_rate, + ) + .context(InvalidIndexOptionSnafu)?, + }, + api::v1::set_index::Options::Inverted(i) => SetIndexOption::Inverted { + column_name: i.column_name, + }, + api::v1::set_index::Options::Skipping(s) => SetIndexOption::Skipping { + column_name: s.column_name, + options: SkippingIndexOptions::new( + s.granularity as u32, + s.false_positive_rate, + as_skipping_index_type( + PbSkippingIndexType::try_from(s.skipping_index_type) + .context(InvalidSetSkippingIndexOptionRequestSnafu)?, + ), + ) + .context(InvalidIndexOptionSnafu)?, + }, + }) +} + +fn unset_index_option_from_proto(unset_index: api::v1::UnsetIndex) -> Result { + let options = unset_index.options.context(MissingAlterIndexOptionSnafu)?; + Ok(match options { + api::v1::unset_index::Options::Fulltext(f) => UnsetIndexOption::Fulltext { + column_name: f.column_name, + }, + api::v1::unset_index::Options::Inverted(i) => UnsetIndexOption::Inverted { + column_name: i.column_name, + }, + api::v1::unset_index::Options::Skipping(s) => UnsetIndexOption::Skipping { + column_name: s.column_name, + }, + }) +} + /// Convert an [`AlterTableExpr`] to an [`AlterTableRequest`] pub fn alter_expr_to_request(table_id: TableId, expr: AlterTableExpr) -> Result { let catalog_name = expr.catalog_name; @@ -121,70 +174,34 @@ pub fn alter_expr_to_request(table_id: TableId, expr: AlterTableExpr) -> Result< .context(InvalidUnsetTableOptionRequestSnafu)?, } } - Kind::SetIndex(o) => match o.options { - Some(opt) => match opt { - api::v1::set_index::Options::Fulltext(f) => AlterKind::SetIndex { - options: SetIndexOptions::Fulltext { - column_name: f.column_name.clone(), - options: FulltextOptions::new( - f.enable, - as_fulltext_option_analyzer( - Analyzer::try_from(f.analyzer) - .context(InvalidSetFulltextOptionRequestSnafu)?, - ), - f.case_sensitive, - as_fulltext_option_backend( - PbFulltextBackend::try_from(f.backend) - .context(InvalidSetFulltextOptionRequestSnafu)?, - ), - f.granularity as u32, - f.false_positive_rate, - ) - .context(InvalidIndexOptionSnafu)?, - }, - }, - api::v1::set_index::Options::Inverted(i) => AlterKind::SetIndex { - options: SetIndexOptions::Inverted { - column_name: i.column_name, - }, - }, - api::v1::set_index::Options::Skipping(s) => AlterKind::SetIndex { - options: SetIndexOptions::Skipping { - column_name: s.column_name, - options: SkippingIndexOptions::new( - s.granularity as u32, - s.false_positive_rate, - as_skipping_index_type( - PbSkippingIndexType::try_from(s.skipping_index_type) - .context(InvalidSetSkippingIndexOptionRequestSnafu)?, - ), - ) - .context(InvalidIndexOptionSnafu)?, - }, - }, - }, - None => return MissingAlterIndexOptionSnafu.fail(), - }, - Kind::UnsetIndex(o) => match o.options { - Some(opt) => match opt { - api::v1::unset_index::Options::Fulltext(f) => AlterKind::UnsetIndex { - options: UnsetIndexOptions::Fulltext { - column_name: f.column_name, - }, - }, - api::v1::unset_index::Options::Inverted(i) => AlterKind::UnsetIndex { - options: UnsetIndexOptions::Inverted { - column_name: i.column_name, - }, - }, - api::v1::unset_index::Options::Skipping(s) => AlterKind::UnsetIndex { - options: UnsetIndexOptions::Skipping { - column_name: s.column_name, - }, - }, - }, - None => return MissingAlterIndexOptionSnafu.fail(), - }, + Kind::SetIndex(o) => { + let option = set_index_option_from_proto(o)?; + AlterKind::SetIndexes { + options: vec![option], + } + } + Kind::UnsetIndex(o) => { + let option = unset_index_option_from_proto(o)?; + AlterKind::UnsetIndexes { + options: vec![option], + } + } + Kind::SetIndexes(o) => { + let options = o + .set_indexes + .into_iter() + .map(set_index_option_from_proto) + .collect::>>()?; + AlterKind::SetIndexes { options } + } + Kind::UnsetIndexes(o) => { + let options = o + .unset_indexes + .into_iter() + .map(unset_index_option_from_proto) + .collect::>>()?; + AlterKind::UnsetIndexes { options } + } Kind::DropDefaults(o) => { let names = o .drop_defaults diff --git a/src/common/meta/src/ddl/alter_table/executor.rs b/src/common/meta/src/ddl/alter_table/executor.rs index 204851b2f3..933d04337e 100644 --- a/src/common/meta/src/ddl/alter_table/executor.rs +++ b/src/common/meta/src/ddl/alter_table/executor.rs @@ -299,8 +299,8 @@ fn build_new_table_info( | AlterKind::ModifyColumnTypes { .. } | AlterKind::SetTableOptions { .. } | AlterKind::UnsetTableOptions { .. } - | AlterKind::SetIndex { .. } - | AlterKind::UnsetIndex { .. } + | AlterKind::SetIndexes { .. } + | AlterKind::UnsetIndexes { .. } | AlterKind::DropDefaults { .. } => {} } diff --git a/src/common/meta/src/ddl/alter_table/region_request.rs b/src/common/meta/src/ddl/alter_table/region_request.rs index c4be75b2f6..dab63cc85a 100644 --- a/src/common/meta/src/ddl/alter_table/region_request.rs +++ b/src/common/meta/src/ddl/alter_table/region_request.rs @@ -108,6 +108,8 @@ fn create_proto_alter_kind( Kind::UnsetTableOptions(v) => Ok(Some(alter_request::Kind::UnsetTableOptions(v.clone()))), Kind::SetIndex(v) => Ok(Some(alter_request::Kind::SetIndex(v.clone()))), Kind::UnsetIndex(v) => Ok(Some(alter_request::Kind::UnsetIndex(v.clone()))), + Kind::SetIndexes(v) => Ok(Some(alter_request::Kind::SetIndexes(v.clone()))), + Kind::UnsetIndexes(v) => Ok(Some(alter_request::Kind::UnsetIndexes(v.clone()))), Kind::DropDefaults(v) => Ok(Some(alter_request::Kind::DropDefaults(v.clone()))), } } diff --git a/src/metric-engine/src/data_region.rs b/src/metric-engine/src/data_region.rs index 80aacc2848..c727f6a5f9 100644 --- a/src/metric-engine/src/data_region.rs +++ b/src/metric-engine/src/data_region.rs @@ -214,8 +214,8 @@ impl DataRegion { match request.kind { AlterKind::SetRegionOptions { options: _ } | AlterKind::UnsetRegionOptions { keys: _ } - | AlterKind::SetIndex { options: _ } - | AlterKind::UnsetIndex { options: _ } => { + | AlterKind::SetIndexes { options: _ } + | AlterKind::UnsetIndexes { options: _ } => { let region_id = utils::to_data_region_id(region_id); self.mito .handle_request(region_id, RegionRequest::Alter(request)) diff --git a/src/mito2/src/engine/alter_test.rs b/src/mito2/src/engine/alter_test.rs index b860cedb4a..1345f4210f 100644 --- a/src/mito2/src/engine/alter_test.rs +++ b/src/mito2/src/engine/alter_test.rs @@ -28,8 +28,8 @@ use store_api::metadata::ColumnMetadata; use store_api::metric_engine_consts::TABLE_COLUMN_METADATA_EXTENSION_KEY; use store_api::region_engine::{RegionEngine, RegionManifestInfo, RegionRole}; use store_api::region_request::{ - AddColumn, AddColumnLocation, AlterKind, ApiSetIndexOptions, RegionAlterRequest, - RegionOpenRequest, RegionRequest, SetRegionOption, + AddColumn, AddColumnLocation, AlterKind, RegionAlterRequest, RegionOpenRequest, RegionRequest, + SetIndexOption, SetRegionOption, }; use store_api::storage::{ColumnId, RegionId, ScanRequest}; @@ -73,18 +73,18 @@ fn add_tag1() -> RegionAlterRequest { fn alter_column_inverted_index() -> RegionAlterRequest { RegionAlterRequest { - kind: AlterKind::SetIndex { - options: ApiSetIndexOptions::Inverted { + kind: AlterKind::SetIndexes { + options: vec![SetIndexOption::Inverted { column_name: "tag_0".to_string(), - }, + }], }, } } fn alter_column_fulltext_options() -> RegionAlterRequest { RegionAlterRequest { - kind: AlterKind::SetIndex { - options: ApiSetIndexOptions::Fulltext { + kind: AlterKind::SetIndexes { + options: vec![SetIndexOption::Fulltext { column_name: "tag_0".to_string(), options: FulltextOptions::new_unchecked( true, @@ -94,7 +94,7 @@ fn alter_column_fulltext_options() -> RegionAlterRequest { 1000, 0.01, ), - }, + }], }, } } diff --git a/src/operator/src/expr_helper.rs b/src/operator/src/expr_helper.rs index 2b2cf7a63e..4fb0b30013 100644 --- a/src/operator/src/expr_helper.rs +++ b/src/operator/src/expr_helper.rs @@ -26,9 +26,9 @@ use api::v1::{ ColumnDataType, ColumnDataTypeExtension, CreateFlowExpr, CreateTableExpr, CreateViewExpr, DropColumn, DropColumns, DropDefaults, ExpireAfter, FulltextBackend as PbFulltextBackend, ModifyColumnType, ModifyColumnTypes, RenameTable, SemanticType, SetDatabaseOptions, - SetFulltext, SetIndex, SetInverted, SetSkipping, SetTableOptions, + SetFulltext, SetIndex, SetIndexes, SetInverted, SetSkipping, SetTableOptions, SkippingIndexType as PbSkippingIndexType, TableName, UnsetDatabaseOptions, UnsetFulltext, - UnsetIndex, UnsetInverted, UnsetSkipping, UnsetTableOptions, + UnsetIndex, UnsetIndexes, UnsetInverted, UnsetSkipping, UnsetTableOptions, }; use common_error::ext::BoxedError; use common_grpc_expr::util::ColumnExpr; @@ -576,64 +576,83 @@ pub(crate) fn to_alter_table_expr( AlterTableOperation::UnsetTableOptions { keys } => { AlterTableKind::UnsetTableOptions(UnsetTableOptions { keys }) } - AlterTableOperation::SetIndex { options } => AlterTableKind::SetIndex(match options { - sql::statements::alter::SetIndexOperation::Fulltext { - column_name, - options, - } => SetIndex { - options: Some(set_index::Options::Fulltext(SetFulltext { - column_name: column_name.value, - enable: options.enable, - analyzer: match options.analyzer { - FulltextAnalyzer::English => Analyzer::English.into(), - FulltextAnalyzer::Chinese => Analyzer::Chinese.into(), - }, - case_sensitive: options.case_sensitive, - backend: match options.backend { - FulltextBackend::Bloom => PbFulltextBackend::Bloom.into(), - FulltextBackend::Tantivy => PbFulltextBackend::Tantivy.into(), - }, - granularity: options.granularity as u64, - false_positive_rate: options.false_positive_rate(), - })), - }, - sql::statements::alter::SetIndexOperation::Inverted { column_name } => SetIndex { - options: Some(set_index::Options::Inverted(SetInverted { - column_name: column_name.value, - })), - }, - sql::statements::alter::SetIndexOperation::Skipping { - column_name, - options, - } => SetIndex { - options: Some(set_index::Options::Skipping(SetSkipping { - column_name: column_name.value, - enable: true, - granularity: options.granularity as u64, - false_positive_rate: options.false_positive_rate(), - skipping_index_type: match options.index_type { - SkippingIndexType::BloomFilter => PbSkippingIndexType::BloomFilter.into(), - }, - })), - }, - }), - AlterTableOperation::UnsetIndex { options } => AlterTableKind::UnsetIndex(match options { - sql::statements::alter::UnsetIndexOperation::Fulltext { column_name } => UnsetIndex { - options: Some(unset_index::Options::Fulltext(UnsetFulltext { - column_name: column_name.value, - })), - }, - sql::statements::alter::UnsetIndexOperation::Inverted { column_name } => UnsetIndex { - options: Some(unset_index::Options::Inverted(UnsetInverted { - column_name: column_name.value, - })), - }, - sql::statements::alter::UnsetIndexOperation::Skipping { column_name } => UnsetIndex { - options: Some(unset_index::Options::Skipping(UnsetSkipping { - column_name: column_name.value, - })), - }, - }), + AlterTableOperation::SetIndex { options } => { + let option = match options { + sql::statements::alter::SetIndexOperation::Fulltext { + column_name, + options, + } => SetIndex { + options: Some(set_index::Options::Fulltext(SetFulltext { + column_name: column_name.value, + enable: options.enable, + analyzer: match options.analyzer { + FulltextAnalyzer::English => Analyzer::English.into(), + FulltextAnalyzer::Chinese => Analyzer::Chinese.into(), + }, + case_sensitive: options.case_sensitive, + backend: match options.backend { + FulltextBackend::Bloom => PbFulltextBackend::Bloom.into(), + FulltextBackend::Tantivy => PbFulltextBackend::Tantivy.into(), + }, + granularity: options.granularity as u64, + false_positive_rate: options.false_positive_rate(), + })), + }, + sql::statements::alter::SetIndexOperation::Inverted { column_name } => SetIndex { + options: Some(set_index::Options::Inverted(SetInverted { + column_name: column_name.value, + })), + }, + sql::statements::alter::SetIndexOperation::Skipping { + column_name, + options, + } => SetIndex { + options: Some(set_index::Options::Skipping(SetSkipping { + column_name: column_name.value, + enable: true, + granularity: options.granularity as u64, + false_positive_rate: options.false_positive_rate(), + skipping_index_type: match options.index_type { + SkippingIndexType::BloomFilter => { + PbSkippingIndexType::BloomFilter.into() + } + }, + })), + }, + }; + AlterTableKind::SetIndexes(SetIndexes { + set_indexes: vec![option], + }) + } + AlterTableOperation::UnsetIndex { options } => { + let option = match options { + sql::statements::alter::UnsetIndexOperation::Fulltext { column_name } => { + UnsetIndex { + options: Some(unset_index::Options::Fulltext(UnsetFulltext { + column_name: column_name.value, + })), + } + } + sql::statements::alter::UnsetIndexOperation::Inverted { column_name } => { + UnsetIndex { + options: Some(unset_index::Options::Inverted(UnsetInverted { + column_name: column_name.value, + })), + } + } + sql::statements::alter::UnsetIndexOperation::Skipping { column_name } => { + UnsetIndex { + options: Some(unset_index::Options::Skipping(UnsetSkipping { + column_name: column_name.value, + })), + } + } + }; + + AlterTableKind::UnsetIndexes(UnsetIndexes { + unset_indexes: vec![option], + }) + } AlterTableOperation::DropDefaults { columns } => { AlterTableKind::DropDefaults(DropDefaults { drop_defaults: columns diff --git a/src/store-api/src/metadata.rs b/src/store-api/src/metadata.rs index c279f92448..eb04378a15 100644 --- a/src/store-api/src/metadata.rs +++ b/src/store-api/src/metadata.rs @@ -29,7 +29,7 @@ use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; use datatypes::arrow; use datatypes::arrow::datatypes::FieldRef; -use datatypes::schema::{ColumnSchema, FulltextOptions, Schema, SchemaRef, SkippingIndexOptions}; +use datatypes::schema::{ColumnSchema, FulltextOptions, Schema, SchemaRef}; use datatypes::types::TimestampType; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize}; @@ -37,8 +37,7 @@ use snafu::{ensure, Location, OptionExt, ResultExt, Snafu}; use crate::codec::PrimaryKeyEncoding; use crate::region_request::{ - AddColumn, AddColumnLocation, AlterKind, ApiSetIndexOptions, ApiUnsetIndexOptions, - ModifyColumnType, + AddColumn, AddColumnLocation, AlterKind, ModifyColumnType, SetIndexOption, UnsetIndexOption, }; use crate::storage::consts::is_internal_column; use crate::storage::{ColumnId, RegionId}; @@ -582,30 +581,8 @@ impl RegionMetadataBuilder { AlterKind::AddColumns { columns } => self.add_columns(columns)?, AlterKind::DropColumns { names } => self.drop_columns(&names), AlterKind::ModifyColumnTypes { columns } => self.modify_column_types(columns)?, - AlterKind::SetIndex { options } => match options { - ApiSetIndexOptions::Fulltext { - column_name, - options, - } => self.change_column_fulltext_options(column_name, true, Some(options))?, - ApiSetIndexOptions::Inverted { column_name } => { - self.change_column_inverted_index_options(column_name, true)? - } - ApiSetIndexOptions::Skipping { - column_name, - options, - } => self.change_column_skipping_index_options(column_name, Some(options))?, - }, - AlterKind::UnsetIndex { options } => match options { - ApiUnsetIndexOptions::Fulltext { column_name } => { - self.change_column_fulltext_options(column_name, false, None)? - } - ApiUnsetIndexOptions::Inverted { column_name } => { - self.change_column_inverted_index_options(column_name, false)? - } - ApiUnsetIndexOptions::Skipping { column_name } => { - self.change_column_skipping_index_options(column_name, None)? - } - }, + AlterKind::SetIndexes { options } => self.set_indexes(options)?, + AlterKind::UnsetIndexes { options } => self.unset_indexes(options)?, AlterKind::SetRegionOptions { options: _ } => { // nothing to be done with RegionMetadata } @@ -741,92 +718,124 @@ impl RegionMetadataBuilder { Ok(()) } - fn change_column_inverted_index_options( - &mut self, - column_name: String, - value: bool, - ) -> Result<()> { - for column_meta in self.column_metadatas.iter_mut() { - if column_meta.column_schema.name == column_name { - column_meta.column_schema.set_inverted_index(value) + fn set_indexes(&mut self, options: Vec) -> Result<()> { + let mut set_index_map: HashMap<_, Vec<_>> = HashMap::new(); + for option in &options { + set_index_map + .entry(option.column_name()) + .or_default() + .push(option); + } + + for column_metadata in self.column_metadatas.iter_mut() { + if let Some(options) = set_index_map.remove(&column_metadata.column_schema.name) { + for option in options { + Self::set_index(column_metadata, option)?; + } } } + Ok(()) } - fn change_column_fulltext_options( - &mut self, - column_name: String, - enable: bool, - options: Option, - ) -> Result<()> { - for column_meta in self.column_metadatas.iter_mut() { - if column_meta.column_schema.name == column_name { + fn unset_indexes(&mut self, options: Vec) -> Result<()> { + let mut unset_index_map: HashMap<_, Vec<_>> = HashMap::new(); + for option in &options { + unset_index_map + .entry(option.column_name()) + .or_default() + .push(option); + } + + for column_metadata in self.column_metadatas.iter_mut() { + if let Some(options) = unset_index_map.remove(&column_metadata.column_schema.name) { + for option in options { + Self::unset_index(column_metadata, option)?; + } + } + } + + Ok(()) + } + + fn set_index(column_metadata: &mut ColumnMetadata, options: &SetIndexOption) -> Result<()> { + match options { + SetIndexOption::Fulltext { + column_name, + options, + } => { ensure!( - column_meta.column_schema.data_type.is_string(), + column_metadata.column_schema.data_type.is_string(), + InvalidColumnOptionSnafu { + column_name, + msg: "FULLTEXT index only supports string type".to_string(), + } + ); + let current_fulltext_options = column_metadata + .column_schema + .fulltext_options() + .with_context(|_| GetFulltextOptionsSnafu { + column_name: column_name.to_string(), + })?; + set_column_fulltext_options( + column_metadata, + column_name, + options, + current_fulltext_options, + )?; + } + SetIndexOption::Inverted { .. } => { + column_metadata.column_schema.set_inverted_index(true) + } + SetIndexOption::Skipping { + column_name, + options, + } => { + column_metadata + .column_schema + .set_skipping_options(options) + .context(UnsetSkippingIndexOptionsSnafu { column_name })?; + } + } + + Ok(()) + } + + fn unset_index(column_metadata: &mut ColumnMetadata, options: &UnsetIndexOption) -> Result<()> { + match options { + UnsetIndexOption::Fulltext { column_name } => { + ensure!( + column_metadata.column_schema.data_type.is_string(), InvalidColumnOptionSnafu { column_name, msg: "FULLTEXT index only supports string type".to_string(), } ); - let current_fulltext_options = column_meta + let current_fulltext_options = column_metadata .column_schema .fulltext_options() - .context(SetFulltextOptionsSnafu { - column_name: column_name.clone(), + .with_context(|_| GetFulltextOptionsSnafu { + column_name: column_name.to_string(), })?; - if enable { - ensure!( - options.is_some(), - InvalidColumnOptionSnafu { - column_name, - msg: "FULLTEXT index options must be provided", - } - ); - set_column_fulltext_options( - column_meta, - column_name, - options.unwrap(), - current_fulltext_options, - )?; - } else { - unset_column_fulltext_options( - column_meta, - column_name, - current_fulltext_options, - )?; - } - break; + unset_column_fulltext_options( + column_metadata, + column_name, + current_fulltext_options, + )?; + } + UnsetIndexOption::Inverted { .. } => { + column_metadata.column_schema.set_inverted_index(false) + } + UnsetIndexOption::Skipping { column_name } => { + column_metadata + .column_schema + .unset_skipping_options() + .context(UnsetSkippingIndexOptionsSnafu { column_name })?; } } - Ok(()) - } - fn change_column_skipping_index_options( - &mut self, - column_name: String, - options: Option, - ) -> Result<()> { - for column_meta in self.column_metadatas.iter_mut() { - if column_meta.column_schema.name == column_name { - if let Some(options) = &options { - column_meta - .column_schema - .set_skipping_options(options) - .context(UnsetSkippingIndexOptionsSnafu { - column_name: column_name.clone(), - })?; - } else { - column_meta.column_schema.unset_skipping_options().context( - UnsetSkippingIndexOptionsSnafu { - column_name: column_name.clone(), - }, - )?; - } - } - } Ok(()) } @@ -1019,6 +1028,14 @@ pub enum MetadataError { location: Location, }, + #[snafu(display("Failed to get fulltext options for column {}", column_name))] + GetFulltextOptions { + column_name: String, + source: datatypes::Error, + #[snafu(implicit)] + location: Location, + }, + #[snafu(display("Failed to set skipping index options for column {}", column_name))] SetSkippingIndexOptions { column_name: String, @@ -1094,8 +1111,8 @@ impl ErrorExt for MetadataError { /// * case_sensitive fn set_column_fulltext_options( column_meta: &mut ColumnMetadata, - column_name: String, - options: FulltextOptions, + column_name: &str, + options: &FulltextOptions, current_options: Option, ) -> Result<()> { if let Some(current_options) = current_options { @@ -1112,7 +1129,7 @@ fn set_column_fulltext_options( column_meta .column_schema - .set_fulltext_options(&options) + .set_fulltext_options(options) .context(SetFulltextOptionsSnafu { column_name })?; Ok(()) @@ -1120,7 +1137,7 @@ fn set_column_fulltext_options( fn unset_column_fulltext_options( column_meta: &mut ColumnMetadata, - column_name: String, + column_name: &str, current_options: Option, ) -> Result<()> { if let Some(mut current_options) = current_options @@ -1625,8 +1642,8 @@ mod test { let mut builder = RegionMetadataBuilder::from_existing(metadata); builder - .alter(AlterKind::SetIndex { - options: ApiSetIndexOptions::Fulltext { + .alter(AlterKind::SetIndexes { + options: vec![SetIndexOption::Fulltext { column_name: "b".to_string(), options: FulltextOptions::new_unchecked( true, @@ -1636,7 +1653,7 @@ mod test { 1000, 0.01, ), - }, + }], }) .unwrap(); let metadata = builder.build().unwrap(); @@ -1656,10 +1673,10 @@ mod test { let mut builder = RegionMetadataBuilder::from_existing(metadata); builder - .alter(AlterKind::UnsetIndex { - options: ApiUnsetIndexOptions::Fulltext { + .alter(AlterKind::UnsetIndexes { + options: vec![UnsetIndexOption::Fulltext { column_name: "b".to_string(), - }, + }], }) .unwrap(); let metadata = builder.build().unwrap(); diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index 6b329ffd8a..75d99299a9 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -27,8 +27,8 @@ use api::v1::region::{ DropRequests, FlushRequest, InsertRequests, OpenRequest, TruncateRequest, }; use api::v1::{ - self, set_index, Analyzer, ArrowIpc, FulltextBackend as PbFulltextBackend, Option as PbOption, - Rows, SemanticType, SkippingIndexType as PbSkippingIndexType, WriteHint, + self, Analyzer, ArrowIpc, FulltextBackend as PbFulltextBackend, Option as PbOption, Rows, + SemanticType, SkippingIndexType as PbSkippingIndexType, WriteHint, }; pub use common_base::AffectedRows; use common_grpc::flight::FlightDecoder; @@ -531,9 +531,9 @@ pub enum AlterKind { /// Unset region options. UnsetRegionOptions { keys: Vec }, /// Set index options. - SetIndex { options: ApiSetIndexOptions }, + SetIndexes { options: Vec }, /// Unset index options. - UnsetIndex { options: ApiUnsetIndexOptions }, + UnsetIndexes { options: Vec }, /// Drop column default value. DropDefaults { /// Name of columns to drop. @@ -542,7 +542,7 @@ pub enum AlterKind { } #[derive(Debug, PartialEq, Eq, Clone)] -pub enum ApiSetIndexOptions { +pub enum SetIndexOption { Fulltext { column_name: String, options: FulltextOptions, @@ -556,49 +556,121 @@ pub enum ApiSetIndexOptions { }, } -impl ApiSetIndexOptions { +impl SetIndexOption { + /// Returns the column name of the index option. pub fn column_name(&self) -> &String { match self { - ApiSetIndexOptions::Fulltext { column_name, .. } => column_name, - ApiSetIndexOptions::Inverted { column_name } => column_name, - ApiSetIndexOptions::Skipping { column_name, .. } => column_name, + SetIndexOption::Fulltext { column_name, .. } => column_name, + SetIndexOption::Inverted { column_name } => column_name, + SetIndexOption::Skipping { column_name, .. } => column_name, } } + /// Returns true if the index option is fulltext. pub fn is_fulltext(&self) -> bool { match self { - ApiSetIndexOptions::Fulltext { .. } => true, - ApiSetIndexOptions::Inverted { .. } => false, - ApiSetIndexOptions::Skipping { .. } => false, + SetIndexOption::Fulltext { .. } => true, + SetIndexOption::Inverted { .. } => false, + SetIndexOption::Skipping { .. } => false, } } } +impl TryFrom for SetIndexOption { + type Error = MetadataError; + + fn try_from(value: v1::SetIndex) -> Result { + let option = value.options.context(InvalidRawRegionRequestSnafu { + err: "missing options in SetIndex", + })?; + + let opt = match option { + v1::set_index::Options::Fulltext(x) => SetIndexOption::Fulltext { + column_name: x.column_name.clone(), + options: FulltextOptions::new( + x.enable, + as_fulltext_option_analyzer( + Analyzer::try_from(x.analyzer).context(DecodeProtoSnafu)?, + ), + x.case_sensitive, + as_fulltext_option_backend( + PbFulltextBackend::try_from(x.backend).context(DecodeProtoSnafu)?, + ), + x.granularity as u32, + x.false_positive_rate, + ) + .context(InvalidIndexOptionSnafu)?, + }, + v1::set_index::Options::Inverted(i) => SetIndexOption::Inverted { + column_name: i.column_name, + }, + v1::set_index::Options::Skipping(s) => SetIndexOption::Skipping { + column_name: s.column_name, + options: SkippingIndexOptions::new( + s.granularity as u32, + s.false_positive_rate, + as_skipping_index_type( + PbSkippingIndexType::try_from(s.skipping_index_type) + .context(DecodeProtoSnafu)?, + ), + ) + .context(InvalidIndexOptionSnafu)?, + }, + }; + + Ok(opt) + } +} + #[derive(Debug, PartialEq, Eq, Clone)] -pub enum ApiUnsetIndexOptions { +pub enum UnsetIndexOption { Fulltext { column_name: String }, Inverted { column_name: String }, Skipping { column_name: String }, } -impl ApiUnsetIndexOptions { +impl UnsetIndexOption { pub fn column_name(&self) -> &String { match self { - ApiUnsetIndexOptions::Fulltext { column_name } => column_name, - ApiUnsetIndexOptions::Inverted { column_name } => column_name, - ApiUnsetIndexOptions::Skipping { column_name } => column_name, + UnsetIndexOption::Fulltext { column_name } => column_name, + UnsetIndexOption::Inverted { column_name } => column_name, + UnsetIndexOption::Skipping { column_name } => column_name, } } pub fn is_fulltext(&self) -> bool { match self { - ApiUnsetIndexOptions::Fulltext { .. } => true, - ApiUnsetIndexOptions::Inverted { .. } => false, - ApiUnsetIndexOptions::Skipping { .. } => false, + UnsetIndexOption::Fulltext { .. } => true, + UnsetIndexOption::Inverted { .. } => false, + UnsetIndexOption::Skipping { .. } => false, } } } +impl TryFrom for UnsetIndexOption { + type Error = MetadataError; + + fn try_from(value: v1::UnsetIndex) -> Result { + let option = value.options.context(InvalidRawRegionRequestSnafu { + err: "missing options in UnsetIndex", + })?; + + let opt = match option { + v1::unset_index::Options::Fulltext(f) => UnsetIndexOption::Fulltext { + column_name: f.column_name, + }, + v1::unset_index::Options::Inverted(i) => UnsetIndexOption::Inverted { + column_name: i.column_name, + }, + v1::unset_index::Options::Skipping(s) => UnsetIndexOption::Skipping { + column_name: s.column_name, + }, + }; + + Ok(opt) + } +} + impl AlterKind { /// Returns an error if the alter kind is invalid. /// @@ -622,19 +694,23 @@ impl AlterKind { } AlterKind::SetRegionOptions { .. } => {} AlterKind::UnsetRegionOptions { .. } => {} - AlterKind::SetIndex { options } => { - Self::validate_column_alter_index_option( - options.column_name(), - metadata, - options.is_fulltext(), - )?; + AlterKind::SetIndexes { options } => { + for option in options { + Self::validate_column_alter_index_option( + option.column_name(), + metadata, + option.is_fulltext(), + )?; + } } - AlterKind::UnsetIndex { options } => { - Self::validate_column_alter_index_option( - options.column_name(), - metadata, - options.is_fulltext(), - )?; + AlterKind::UnsetIndexes { options } => { + for option in options { + Self::validate_column_alter_index_option( + option.column_name(), + metadata, + option.is_fulltext(), + )?; + } } AlterKind::DropDefaults { names } => { names @@ -664,12 +740,12 @@ impl AlterKind { true } AlterKind::UnsetRegionOptions { .. } => true, - AlterKind::SetIndex { options, .. } => { - metadata.column_by_name(options.column_name()).is_some() - } - AlterKind::UnsetIndex { options } => { - metadata.column_by_name(options.column_name()).is_some() - } + AlterKind::SetIndexes { options, .. } => options + .iter() + .any(|option| metadata.column_by_name(option.column_name()).is_some()), + AlterKind::UnsetIndexes { options } => options + .iter() + .any(|option| metadata.column_by_name(option.column_name()).is_some()), AlterKind::DropDefaults { names } => names .iter() .any(|name| metadata.column_by_name(name).is_some()), @@ -760,61 +836,25 @@ impl TryFrom for AlterKind { .map(|key| UnsetRegionOption::try_from(key.as_str())) .collect::>>()?, }, - alter_request::Kind::SetIndex(o) => match o.options.unwrap() { - set_index::Options::Fulltext(x) => AlterKind::SetIndex { - options: ApiSetIndexOptions::Fulltext { - column_name: x.column_name.clone(), - options: FulltextOptions::new( - x.enable, - as_fulltext_option_analyzer( - Analyzer::try_from(x.analyzer).context(DecodeProtoSnafu)?, - ), - x.case_sensitive, - as_fulltext_option_backend( - PbFulltextBackend::try_from(x.backend).context(DecodeProtoSnafu)?, - ), - x.granularity as u32, - x.false_positive_rate, - ) - .context(InvalidIndexOptionSnafu)?, - }, - }, - set_index::Options::Inverted(i) => AlterKind::SetIndex { - options: ApiSetIndexOptions::Inverted { - column_name: i.column_name, - }, - }, - set_index::Options::Skipping(s) => AlterKind::SetIndex { - options: ApiSetIndexOptions::Skipping { - column_name: s.column_name, - options: SkippingIndexOptions::new( - s.granularity as u32, - s.false_positive_rate, - as_skipping_index_type( - PbSkippingIndexType::try_from(s.skipping_index_type) - .context(DecodeProtoSnafu)?, - ), - ) - .context(InvalidIndexOptionSnafu)?, - }, - }, + alter_request::Kind::SetIndex(o) => AlterKind::SetIndexes { + options: vec![SetIndexOption::try_from(o)?], }, - alter_request::Kind::UnsetIndex(o) => match o.options.unwrap() { - v1::unset_index::Options::Fulltext(f) => AlterKind::UnsetIndex { - options: ApiUnsetIndexOptions::Fulltext { - column_name: f.column_name, - }, - }, - v1::unset_index::Options::Inverted(i) => AlterKind::UnsetIndex { - options: ApiUnsetIndexOptions::Inverted { - column_name: i.column_name, - }, - }, - v1::unset_index::Options::Skipping(s) => AlterKind::UnsetIndex { - options: ApiUnsetIndexOptions::Skipping { - column_name: s.column_name, - }, - }, + alter_request::Kind::UnsetIndex(o) => AlterKind::UnsetIndexes { + options: vec![UnsetIndexOption::try_from(o)?], + }, + alter_request::Kind::SetIndexes(o) => AlterKind::SetIndexes { + options: o + .set_indexes + .into_iter() + .map(SetIndexOption::try_from) + .collect::>>()?, + }, + alter_request::Kind::UnsetIndexes(o) => AlterKind::UnsetIndexes { + options: o + .unset_indexes + .into_iter() + .map(UnsetIndexOption::try_from) + .collect::>>()?, }, alter_request::Kind::DropDefaults(x) => AlterKind::DropDefaults { names: x.drop_defaults.into_iter().map(|x| x.column_name).collect(), @@ -1650,8 +1690,8 @@ mod tests { #[test] fn test_validate_modify_column_fulltext_options() { - let kind = AlterKind::SetIndex { - options: ApiSetIndexOptions::Fulltext { + let kind = AlterKind::SetIndexes { + options: vec![SetIndexOption::Fulltext { column_name: "tag_0".to_string(), options: FulltextOptions::new_unchecked( true, @@ -1661,17 +1701,17 @@ mod tests { 1000, 0.01, ), - }, + }], }; let request = RegionAlterRequest { kind }; let mut metadata = new_metadata(); metadata.schema_version = 1; request.validate(&metadata).unwrap(); - let kind = AlterKind::UnsetIndex { - options: ApiUnsetIndexOptions::Fulltext { + let kind = AlterKind::UnsetIndexes { + options: vec![UnsetIndexOption::Fulltext { column_name: "tag_0".to_string(), - }, + }], }; let request = RegionAlterRequest { kind }; let mut metadata = new_metadata(); diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs index 39083cfec8..3f1ca58f03 100644 --- a/src/table/src/metadata.rs +++ b/src/table/src/metadata.rs @@ -35,8 +35,8 @@ use store_api::storage::{ColumnDescriptor, ColumnDescriptorBuilder, ColumnId, Re use crate::error::{self, Result}; use crate::requests::{ - AddColumnRequest, AlterKind, ModifyColumnTypeRequest, SetIndexOptions, TableOptions, - UnsetIndexOptions, + AddColumnRequest, AlterKind, ModifyColumnTypeRequest, SetIndexOption, TableOptions, + UnsetIndexOption, }; use crate::table_reference::TableReference; @@ -236,39 +236,8 @@ impl TableMeta { AlterKind::RenameTable { .. } => Ok(self.new_meta_builder()), AlterKind::SetTableOptions { options } => self.set_table_options(options), AlterKind::UnsetTableOptions { keys } => self.unset_table_options(keys), - AlterKind::SetIndex { options } => match options { - SetIndexOptions::Fulltext { - column_name, - options, - } => self.change_column_fulltext_options( - table_name, - column_name, - true, - Some(options), - ), - SetIndexOptions::Inverted { column_name } => { - self.change_column_modify_inverted_index(table_name, column_name, true) - } - SetIndexOptions::Skipping { - column_name, - options, - } => self.change_column_skipping_index_options( - table_name, - column_name, - Some(options), - ), - }, - AlterKind::UnsetIndex { options } => match options { - UnsetIndexOptions::Fulltext { column_name } => { - self.change_column_fulltext_options(table_name, column_name, false, None) - } - UnsetIndexOptions::Inverted { column_name } => { - self.change_column_modify_inverted_index(table_name, column_name, false) - } - UnsetIndexOptions::Skipping { column_name } => { - self.change_column_skipping_index_options(table_name, column_name, None) - } - }, + AlterKind::SetIndexes { options } => self.set_indexes(table_name, options), + AlterKind::UnsetIndexes { options } => self.unset_indexes(table_name, options), AlterKind::DropDefaults { names } => self.drop_defaults(table_name, names), } } @@ -310,30 +279,38 @@ impl TableMeta { self.set_table_options(&requests) } - /// Creates a [TableMetaBuilder] with modified column inverted index. - fn change_column_modify_inverted_index( + fn set_indexes( &self, table_name: &str, - column_name: &str, - value: bool, + requests: &[SetIndexOption], ) -> Result { let table_schema = &self.schema; - let mut meta_builder = self.new_meta_builder(); - - let mut columns: Vec = - Vec::with_capacity(table_schema.column_schemas().len()); - - for column_schema in table_schema.column_schemas().iter() { - if column_schema.name == column_name { - let mut new_column_schema = column_schema.clone(); - new_column_schema.set_inverted_index(value); - columns.push(new_column_schema); - } else { - columns.push(column_schema.clone()); - } + let mut set_index_options: HashMap<&str, Vec<_>> = HashMap::new(); + for request in requests { + let column_name = request.column_name(); + table_schema + .column_index_by_name(column_name) + .with_context(|| error::ColumnNotExistsSnafu { + column_name, + table_name, + })?; + set_index_options + .entry(column_name) + .or_default() + .push(request); + } + + let mut meta_builder = self.new_meta_builder(); + let mut columns: Vec<_> = Vec::with_capacity(table_schema.column_schemas().len()); + for mut column in table_schema.column_schemas().iter().cloned() { + if let Some(request) = set_index_options.get(column.name.as_str()) { + for request in request { + self.set_index(&mut column, request)?; + } + } + columns.push(column); } - // 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}"), @@ -344,12 +321,17 @@ impl TableMeta { 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 new_schema = builder.build().with_context(|_| { + let column_names = requests + .iter() + .map(|request| request.column_name()) + .collect::>(); + error::SchemaBuildSnafu { + msg: format!( + "Table {table_name} cannot set index options with columns {column_names:?}", + ), + } })?; - let _ = meta_builder .schema(Arc::new(new_schema)) .primary_key_indices(self.primary_key_indices.clone()); @@ -357,68 +339,38 @@ impl TableMeta { Ok(meta_builder) } - /// Creates a [TableMetaBuilder] with modified column fulltext options. - fn change_column_fulltext_options( + fn unset_indexes( &self, table_name: &str, - column_name: &str, - enable: bool, - options: Option<&FulltextOptions>, + requests: &[UnsetIndexOption], ) -> Result { 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::InvalidColumnOptionSnafu { - column_name, - msg: "FULLTEXT index only supports string type", - } - ); - - let current_fulltext_options = column - .fulltext_options() - .context(error::SetFulltextOptionsSnafu { column_name })?; - - let mut columns = Vec::with_capacity(table_schema.column_schemas().len()); - for column_schema in table_schema.column_schemas() { - if column_schema.name == column_name { - let mut new_column_schema = column_schema.clone(); - if enable { - ensure!( - options.is_some(), - error::InvalidColumnOptionSnafu { - column_name, - msg: "FULLTEXT index options must be provided", - } - ); - set_column_fulltext_options( - &mut new_column_schema, - column_name, - options.unwrap(), - current_fulltext_options.clone(), - )? - } else { - unset_column_fulltext_options( - &mut new_column_schema, - column_name, - current_fulltext_options.clone(), - )? - } - columns.push(new_column_schema); - } else { - columns.push(column_schema.clone()); - } + let mut set_index_options: HashMap<&str, Vec<_>> = HashMap::new(); + for request in requests { + let column_name = request.column_name(); + table_schema + .column_index_by_name(column_name) + .with_context(|| error::ColumnNotExistsSnafu { + column_name, + table_name, + })?; + set_index_options + .entry(column_name) + .or_default() + .push(request); + } + + let mut meta_builder = self.new_meta_builder(); + let mut columns: Vec<_> = Vec::with_capacity(table_schema.column_schemas().len()); + for mut column in table_schema.column_schemas().iter().cloned() { + if let Some(request) = set_index_options.get(column.name.as_str()) { + for request in request { + self.unset_index(&mut column, request)?; + } + } + columns.push(column); } - // 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}"), @@ -429,12 +381,17 @@ impl TableMeta { 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 new_schema = builder.build().with_context(|_| { + let column_names = requests + .iter() + .map(|request| request.column_name()) + .collect::>(); + error::SchemaBuildSnafu { + msg: format!( + "Table {table_name} cannot set index options with columns {column_names:?}", + ), + } })?; - let _ = meta_builder .schema(Arc::new(new_schema)) .primary_key_indices(self.primary_key_indices.clone()); @@ -442,54 +399,70 @@ impl TableMeta { Ok(meta_builder) } - /// Creates a [TableMetaBuilder] with modified column skipping index options. - fn change_column_skipping_index_options( - &self, - table_name: &str, - column_name: &str, - options: Option<&SkippingIndexOptions>, - ) -> Result { - let table_schema = &self.schema; - let mut meta_builder = self.new_meta_builder(); - - let mut columns = Vec::with_capacity(table_schema.column_schemas().len()); - for column_schema in table_schema.column_schemas() { - if column_schema.name == column_name { - let mut new_column_schema = column_schema.clone(); - if let Some(options) = options { - set_column_skipping_index_options( - &mut new_column_schema, + fn set_index(&self, column_schema: &mut ColumnSchema, request: &SetIndexOption) -> Result<()> { + match request { + SetIndexOption::Fulltext { + column_name, + options, + } => { + ensure!( + column_schema.data_type.is_string(), + error::InvalidColumnOptionSnafu { column_name, - options, - )?; - } else { - unset_column_skipping_index_options(&mut new_column_schema, column_name)?; - } - columns.push(new_column_schema); - } else { - columns.push(column_schema.clone()); + msg: "FULLTEXT index only supports string type", + } + ); + + let current_fulltext_options = column_schema + .fulltext_options() + .context(error::SetFulltextOptionsSnafu { column_name })?; + set_column_fulltext_options( + column_schema, + column_name, + options, + current_fulltext_options, + )?; + } + SetIndexOption::Inverted { column_name } => { + debug_assert_eq!(column_schema.name, *column_name); + column_schema.set_inverted_index(true); + } + SetIndexOption::Skipping { + column_name, + options, + } => { + set_column_skipping_index_options(column_schema, column_name, options)?; } } - 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); + Ok(()) + } - for (k, v) in table_schema.metadata().iter() { - builder = builder.add_metadata(k, v); + fn unset_index( + &self, + column_schema: &mut ColumnSchema, + request: &UnsetIndexOption, + ) -> Result<()> { + match request { + UnsetIndexOption::Fulltext { column_name } => { + let current_fulltext_options = column_schema + .fulltext_options() + .context(error::SetFulltextOptionsSnafu { column_name })?; + unset_column_fulltext_options( + column_schema, + column_name, + current_fulltext_options.clone(), + )? + } + UnsetIndexOption::Inverted { .. } => { + column_schema.set_inverted_index(false); + } + UnsetIndexOption::Skipping { column_name } => { + unset_column_skipping_index_options(column_schema, column_name)?; + } } - let new_schema = builder.build().with_context(|_| error::SchemaBuildSnafu { - msg: format!("Failed to convert column schemas into schema for table {table_name}"), - })?; - - let _ = meta_builder - .schema(Arc::new(new_schema)) - .primary_key_indices(self.primary_key_indices.clone()); - - Ok(meta_builder) + Ok(()) } // TODO(yingwen): Remove this. @@ -1975,11 +1948,11 @@ mod tests { .build() .unwrap(); - let alter_kind = AlterKind::SetIndex { - options: SetIndexOptions::Fulltext { + let alter_kind = AlterKind::SetIndexes { + options: vec![SetIndexOption::Fulltext { column_name: "col1".to_string(), options: FulltextOptions::default(), - }, + }], }; let err = meta .builder_with_alter_kind("my_table", &alter_kind) @@ -1994,8 +1967,8 @@ mod tests { let new_meta = add_columns_to_meta_with_location(&meta); assert_eq!(meta.region_numbers, new_meta.region_numbers); - let alter_kind = AlterKind::SetIndex { - options: SetIndexOptions::Fulltext { + let alter_kind = AlterKind::SetIndexes { + options: vec![SetIndexOption::Fulltext { column_name: "my_tag_first".to_string(), options: FulltextOptions::new_unchecked( true, @@ -2005,7 +1978,7 @@ mod tests { 1000, 0.01, ), - }, + }], }; let new_meta = new_meta .builder_with_alter_kind("my_table", &alter_kind) @@ -2024,10 +1997,10 @@ mod tests { ); assert!(fulltext_options.case_sensitive); - let alter_kind = AlterKind::UnsetIndex { - options: UnsetIndexOptions::Fulltext { + let alter_kind = AlterKind::UnsetIndexes { + options: vec![UnsetIndexOption::Fulltext { column_name: "my_tag_first".to_string(), - }, + }], }; let new_meta = new_meta .builder_with_alter_kind("my_table", &alter_kind) diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 7cbf5b3e1f..d176d043ca 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -251,11 +251,11 @@ pub enum AlterKind { UnsetTableOptions { keys: Vec, }, - SetIndex { - options: SetIndexOptions, + SetIndexes { + options: Vec, }, - UnsetIndex { - options: UnsetIndexOptions, + UnsetIndexes { + options: Vec, }, DropDefaults { names: Vec, @@ -263,7 +263,7 @@ pub enum AlterKind { } #[derive(Debug, Clone, Serialize, Deserialize)] -pub enum SetIndexOptions { +pub enum SetIndexOption { Fulltext { column_name: String, options: FulltextOptions, @@ -277,13 +277,35 @@ pub enum SetIndexOptions { }, } +impl SetIndexOption { + /// Returns the column name of the index option. + pub fn column_name(&self) -> &str { + match self { + SetIndexOption::Fulltext { column_name, .. } => column_name, + SetIndexOption::Inverted { column_name, .. } => column_name, + SetIndexOption::Skipping { column_name, .. } => column_name, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] -pub enum UnsetIndexOptions { +pub enum UnsetIndexOption { Fulltext { column_name: String }, Inverted { column_name: String }, Skipping { column_name: String }, } +impl UnsetIndexOption { + /// Returns the column name of the index option. + pub fn column_name(&self) -> &str { + match self { + UnsetIndexOption::Fulltext { column_name, .. } => column_name, + UnsetIndexOption::Inverted { column_name, .. } => column_name, + UnsetIndexOption::Skipping { column_name, .. } => column_name, + } + } +} + #[derive(Debug)] pub struct InsertRequest { pub catalog_name: String,