feat: support alter inverted index. starting with parser and refactor…

…ing the proto
GreptimeTeam · Dec 10, 2024 · 0313c65 · 0313c65
1 parent 84a10a9
commit 0313c65
Show file tree

Hide file tree

Showing 15 changed files with 483 additions and 252 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -122,7 +122,7 @@ etcd-client = "0.13"
 fst = "0.4.7"
 futures = "0.3"
 futures-util = "0.3"
-greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "e77db40c93a46efdd4d0ca39b4de5620c3d650e4" }
+greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "5af5b4e883919f2f7e22ffdea13b656c3ca217b8" }
 hex = "0.4"
 humantime = "2.1"
 humantime-serde = "1.1"

diff --git a/src/common/grpc-expr/src/alter.rs b/src/common/grpc-expr/src/alter.rs
@@ -25,7 +25,10 @@ use datatypes::schema::{ColumnSchema, FulltextOptions, RawSchema};
 use snafu::{ensure, OptionExt, ResultExt};
 use store_api::region_request::{SetRegionOption, UnsetRegionOption};
 use table::metadata::TableId;
-use table::requests::{AddColumnRequest, AlterKind, AlterTableRequest, ModifyColumnTypeRequest};
+use table::requests::{
+    AddColumnRequest, AlterKind, AlterTableRequest, ModifyColumnTypeRequest, SetIndexOptions,
+    UnsetIndexOptions,
+};
 
 use crate::error::{
     InvalidColumnDefSnafu, InvalidSetFulltextOptionRequestSnafu, InvalidSetTableOptionRequestSnafu,
@@ -113,24 +116,37 @@ pub fn alter_expr_to_request(table_id: TableId, expr: AlterTableExpr) -> Result<
                     .context(InvalidUnsetTableOptionRequestSnafu)?,
             }
         }
-        Kind::SetColumnFulltext(c) => AlterKind::SetColumnFulltext {
-            column_name: c.column_name,
-            options: FulltextOptions {
-                enable: c.enable,
-                analyzer: as_fulltext_option(
-                    Analyzer::try_from(c.analyzer).context(InvalidSetFulltextOptionRequestSnafu)?,
-                ),
-                case_sensitive: c.case_sensitive,
+        Kind::SetIndex(o) => match o.options.unwrap() {
+            api::v1::set_index::Options::Fulltext(f) => AlterKind::SetIndex {
+                options: SetIndexOptions::Fulltext {
+                    column_name: f.column_name.clone(),
+                    options: FulltextOptions {
+                        enable: f.enable,
+                        analyzer: as_fulltext_option(
+                            Analyzer::try_from(f.analyzer)
+                                .context(InvalidSetFulltextOptionRequestSnafu)?,
+                        ),
+                        case_sensitive: f.case_sensitive,
+                    },
+                },
+            },
+            api::v1::set_index::Options::Inverted(i) => AlterKind::SetIndex {
+                options: SetIndexOptions::Inverted {
+                    column_name: i.column_name,
+                },
             },
         },
-        Kind::UnsetColumnFulltext(c) => AlterKind::UnsetColumnFulltext {
-            column_name: c.column_name,
-        },
-        Kind::SetColumnInvertedIndex(c) => AlterKind::SetColumnInvertedIndex {
-            column_name: c.column_name,
-        },
-        Kind::UnsetColumnInvertedIndex(c) => AlterKind::UnsetColumnInvertedIndex {
-            column_name: c.column_name,
+        Kind::UnsetIndex(o) => match o.options.unwrap() {
+            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,
+                },
+            },
         },
     };
 

diff --git a/src/common/meta/src/ddl/alter_table/region_request.rs b/src/common/meta/src/ddl/alter_table/region_request.rs
@@ -108,10 +108,8 @@ fn create_proto_alter_kind(
         Kind::RenameTable(_) => Ok(None),
         Kind::SetTableOptions(v) => Ok(Some(alter_request::Kind::SetTableOptions(v.clone()))),
         Kind::UnsetTableOptions(v) => Ok(Some(alter_request::Kind::UnsetTableOptions(v.clone()))),
-        Kind::SetColumnFulltext(v) => Ok(Some(alter_request::Kind::SetColumnFulltext(v.clone()))),
-        Kind::UnsetColumnFulltext(v) => {
-            Ok(Some(alter_request::Kind::UnsetColumnFulltext(v.clone())))
-        }
+        Kind::SetIndex(v) => Ok(Some(alter_request::Kind::SetIndex(v.clone()))),
+        Kind::UnsetIndex(v) => Ok(Some(alter_request::Kind::UnsetIndex(v.clone()))),
     }
 }
 

diff --git a/src/common/meta/src/ddl/alter_table/update_metadata.rs b/src/common/meta/src/ddl/alter_table/update_metadata.rs
@@ -55,10 +55,8 @@ impl AlterTableProcedure {
             | AlterKind::ModifyColumnTypes { .. }
             | AlterKind::SetTableOptions { .. }
             | AlterKind::UnsetTableOptions { .. }
-            | AlterKind::SetColumnFulltext { .. }
-            | AlterKind::UnsetColumnFulltext { .. }
-            | AlterKind::SetColumnInvertedIndex { .. }
-            | AlterKind::UnsetColumnInvertedIndex { .. } => {}
+            | AlterKind::SetIndex { .. }
+            | AlterKind::UnsetIndex { .. } => {}
         }
 
         Ok(new_info)

diff --git a/src/datatypes/src/schema/column_schema.rs b/src/datatypes/src/schema/column_schema.rs
@@ -149,6 +149,19 @@ impl ColumnSchema {
         self
     }
 
+    pub fn update_inverted_index(&mut self, enabled: bool) {
+        match enabled {
+            true => {
+                self.metadata
+                    .insert(INVERTED_INDEX_KEY.to_string(), enabled.to_string());
+            }
+
+            false => {
+                self.metadata.remove(INVERTED_INDEX_KEY);
+            }
+        }
+    }
+
     pub fn is_inverted_indexed(&self) -> bool {
         self.metadata
             .get(INVERTED_INDEX_KEY)

diff --git a/src/mito2/src/engine/alter_test.rs b/src/mito2/src/engine/alter_test.rs
@@ -26,8 +26,8 @@ use datatypes::schema::{ColumnSchema, FulltextAnalyzer, FulltextOptions};
 use store_api::metadata::ColumnMetadata;
 use store_api::region_engine::{RegionEngine, RegionRole};
 use store_api::region_request::{
-    AddColumn, AddColumnLocation, AlterKind, RegionAlterRequest, RegionOpenRequest, RegionRequest,
-    SetRegionOption,
+    AddColumn, AddColumnLocation, AlterKind, ApiSetIndexOptions, RegionAlterRequest,
+    RegionOpenRequest, RegionRequest, SetRegionOption,
 };
 use store_api::storage::{RegionId, ScanRequest};
 
@@ -72,12 +72,14 @@ fn add_tag1() -> RegionAlterRequest {
 fn alter_column_fulltext_options() -> RegionAlterRequest {
     RegionAlterRequest {
         schema_version: 0,
-        kind: AlterKind::SetColumnFulltext {
-            column_name: "tag_0".to_string(),
-            options: FulltextOptions {
-                enable: true,
-                analyzer: FulltextAnalyzer::English,
-                case_sensitive: false,
+        kind: AlterKind::SetIndex {
+            options: ApiSetIndexOptions::Fulltext {
+                column_name: "tag_0".to_string(),
+                options: FulltextOptions {
+                    enable: true,
+                    analyzer: FulltextAnalyzer::English,
+                    case_sensitive: false,
+                },
             },
         },
     }

diff --git a/src/operator/src/expr_factory.rs b/src/operator/src/expr_factory.rs
@@ -19,11 +19,11 @@ use api::v1::alter_database_expr::Kind as AlterDatabaseKind;
 use api::v1::alter_table_expr::Kind as AlterTableKind;
 use api::v1::column_def::options_from_column_schema;
 use api::v1::{
-    AddColumn, AddColumns, AlterDatabaseExpr, AlterTableExpr, Analyzer, ColumnDataType,
-    ColumnDataTypeExtension, CreateFlowExpr, CreateTableExpr, CreateViewExpr, DropColumn,
-    DropColumns, ExpireAfter, ModifyColumnType, ModifyColumnTypes, RenameTable, SemanticType,
-    SetColumnFulltext, SetDatabaseOptions, SetTableOptions, TableName, UnsetColumnFulltext,
-    UnsetDatabaseOptions, UnsetTableOptions,
+    set_index, unset_index, AddColumn, AddColumns, AlterDatabaseExpr, AlterTableExpr, Analyzer,
+    ColumnDataType, ColumnDataTypeExtension, CreateFlowExpr, CreateTableExpr, CreateViewExpr,
+    DropColumn, DropColumns, ExpireAfter, ModifyColumnType, ModifyColumnTypes, RenameTable,
+    SemanticType, SetDatabaseOptions, SetFulltext, SetIndex, SetInverted, SetTableOptions,
+    TableName, UnsetDatabaseOptions, UnsetFulltext, UnsetIndex, UnsetInverted, UnsetTableOptions,
 };
 use common_error::ext::BoxedError;
 use common_grpc_expr::util::ColumnExpr;
@@ -540,39 +540,39 @@ pub(crate) fn to_alter_table_expr(
         AlterTableOperation::UnsetTableOptions { keys } => {
             AlterTableKind::UnsetTableOptions(UnsetTableOptions { keys })
         }
-        AlterTableOperation::SetColumnFulltext {
-            column_name,
-            options,
-        } => AlterTableKind::SetColumnFulltext(SetColumnFulltext {
-            column_name: column_name.value,
-            enable: options.enable,
-            analyzer: match options.analyzer {
-                FulltextAnalyzer::English => Analyzer::English.into(),
-                FulltextAnalyzer::Chinese => Analyzer::Chinese.into(),
+        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,
+                })),
+            },
+            sql::statements::alter::SetIndexOperation::Inverted { column_name } => SetIndex {
+                options: Some(set_index::Options::Inverted(SetInverted {
+                    column_name: column_name.value,
+                })),
             },
-            case_sensitive: options.case_sensitive,
         }),
-        AlterTableOperation::UnsetColumnFulltext { column_name } => {
-            AlterTableKind::UnsetColumnFulltext(UnsetColumnFulltext {
-                column_name: column_name.value,
-            })
-        },
-        AlterTableOperation::SetColumnInvertedIndex {
-            column_name,
-        } => AlterTableKind::SetColumnFulltext(SetColumnFulltext {
-            column_name: column_name.value,
-            enable: options.enable,
-            analyzer: match options.analyzer {
-                FulltextAnalyzer::English => Analyzer::English.into(),
-                FulltextAnalyzer::Chinese => Analyzer::Chinese.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,
+                })),
             },
-            case_sensitive: options.case_sensitive,
         }),
-        AlterTableOperation::UnsetColumnFulltext { column_name } => {
-            AlterTableKind::UnsetColumnFulltext(UnsetColumnFulltext {
-                column_name: column_name.value,
-            })
-        }
     };
 
     Ok(AlterTableExpr {

diff --git a/src/sql/src/parser.rs b/src/sql/src/parser.rs
@@ -215,6 +215,7 @@ impl ParserContext<'_> {
 
     // Report unexpected token
     pub(crate) fn expected<T>(&self, expected: &str, found: TokenWithLocation) -> Result<T> {
+        println!("verify  ++++ {:?}", found.token);
         Err(ParserError::ParserError(format!(
             "Expected {expected}, found: {found}",
         )))