From 02824e37fba4bcb2cef6effc0d927174858a3eac Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Fri, 13 Dec 2024 10:30:32 +0800 Subject: [PATCH 1/6] skip index parser Signed-off-by: Ruihang Xia --- src/datatypes/src/schema.rs | 4 +- src/datatypes/src/schema/column_schema.rs | 4 + src/sql/src/parsers/create_parser.rs | 93 ++++++++++++++++++++++- src/sql/src/parsers/utils.rs | 13 +++- src/sql/src/statements.rs | 1 + src/sql/src/statements/create.rs | 11 +++ 6 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/datatypes/src/schema.rs b/src/datatypes/src/schema.rs index 2eaa0254fbee..d85f4371c1b6 100644 --- a/src/datatypes/src/schema.rs +++ b/src/datatypes/src/schema.rs @@ -30,8 +30,8 @@ use crate::prelude::ConcreteDataType; pub use crate::schema::column_schema::{ ColumnSchema, FulltextAnalyzer, FulltextOptions, Metadata, COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, - COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COMMENT_KEY, FULLTEXT_KEY, INVERTED_INDEX_KEY, - TIME_INDEX_KEY, + COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIP_INDEX_OPT_KEY_TYPE, COMMENT_KEY, FULLTEXT_KEY, INVERTED_INDEX_KEY, TIME_INDEX_KEY, }; pub use crate::schema::constraint::ColumnDefaultConstraint; pub use crate::schema::raw::RawSchema; diff --git a/src/datatypes/src/schema/column_schema.rs b/src/datatypes/src/schema/column_schema.rs index c1e2df846918..aebbc862aaf0 100644 --- a/src/datatypes/src/schema/column_schema.rs +++ b/src/datatypes/src/schema/column_schema.rs @@ -45,6 +45,10 @@ pub const COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE: &str = "enable"; pub const COLUMN_FULLTEXT_OPT_KEY_ANALYZER: &str = "analyzer"; pub const COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE: &str = "case_sensitive"; +/// Keys used in SKIP index options +pub const COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY: &str = "granularity"; +pub const COLUMN_SKIP_INDEX_OPT_KEY_TYPE: &str = "type"; + /// Schema of a column, used as an immutable struct. #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ColumnSchema { diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index bb9aadadb703..83ffa0e502d4 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -36,7 +36,9 @@ use crate::error::{ SyntaxSnafu, UnexpectedSnafu, UnsupportedSnafu, }; use crate::parser::{ParserContext, FLOW}; -use crate::parsers::utils::validate_column_fulltext_create_option; +use crate::parsers::utils::{ + validate_column_fulltext_create_option, validate_column_skip_index_create_option, +}; use crate::statements::create::{ Column, ColumnExtensions, CreateDatabase, CreateExternalTable, CreateFlow, CreateTable, CreateTableLike, CreateView, Partitions, TableConstraint, VECTOR_OPT_DIM, @@ -701,6 +703,38 @@ impl<'a> ParserContext<'a> { column_extensions.vector_options = Some(options.into()); } + let mut is_index_declared = false; + + if parser.parse_keyword(Keyword::SKIP) { + ensure!( + column_extensions.skip_index_options.is_none(), + InvalidColumnOptionSnafu { + name: column_name.to_string(), + msg: "duplicated SKIP index option", + } + ); + + let options = parser + .parse_options(Keyword::WITH) + .context(error::SyntaxSnafu)? + .into_iter() + .map(parse_option_string) + .collect::>>()?; + + for key in options.keys() { + ensure!( + validate_column_skip_index_create_option(key), + InvalidColumnOptionSnafu { + name: column_name.to_string(), + msg: format!("invalid SKIP option: {key}"), + } + ); + } + + column_extensions.skip_index_options = Some(options.into()); + is_index_declared |= true; + } + if parser.parse_keyword(Keyword::FULLTEXT) { ensure!( column_extensions.fulltext_options.is_none(), @@ -738,10 +772,10 @@ impl<'a> ParserContext<'a> { } column_extensions.fulltext_options = Some(options.into()); - Ok(true) - } else { - Ok(false) + is_index_declared |= true; } + + Ok(is_index_declared) } fn parse_optional_table_constraint(&mut self) -> Result> { @@ -2103,6 +2137,57 @@ CREATE TABLE log ( .contains("invalid FULLTEXT option")); } + #[test] + fn test_parse_create_table_skip_options() { + let sql = r" +CREATE TABLE log ( + ts TIMESTAMP TIME INDEX, + msg INT SKIP WITH (granularity='8192', type='bloom'), +)"; + let result = + ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) + .unwrap(); + + if let Statement::CreateTable(c) = &result[0] { + c.columns.iter().for_each(|col| { + if col.name().value == "msg" { + assert!(!col + .extensions + .skip_index_options + .as_ref() + .unwrap() + .is_empty()); + } + }); + } else { + panic!("should be create_table statement"); + } + + let sql = r" + CREATE TABLE log ( + ts TIMESTAMP TIME INDEX, + msg INT SKIP, + )"; + let result = + ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) + .unwrap(); + + if let Statement::CreateTable(c) = &result[0] { + c.columns.iter().for_each(|col| { + if col.name().value == "msg" { + assert!(col + .extensions + .skip_index_options + .as_ref() + .unwrap() + .is_empty()); + } + }); + } else { + panic!("should be create_table statement"); + } + } + #[test] fn test_parse_create_view_with_columns() { let sql = "CREATE VIEW test () AS SELECT * FROM NUMBERS"; diff --git a/src/sql/src/parsers/utils.rs b/src/sql/src/parsers/utils.rs index ae5146d7ee7b..ef281b50ac38 100644 --- a/src/sql/src/parsers/utils.rs +++ b/src/sql/src/parsers/utils.rs @@ -26,7 +26,10 @@ use datafusion_expr::{AggregateUDF, ScalarUDF, TableSource, WindowUDF}; use datafusion_sql::planner::{ContextProvider, SqlToRel}; use datafusion_sql::TableReference; use datatypes::arrow::datatypes::DataType; -use datatypes::schema::{COLUMN_FULLTEXT_OPT_KEY_ANALYZER, COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE}; +use datatypes::schema::{ + COLUMN_FULLTEXT_OPT_KEY_ANALYZER, COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, + COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, COLUMN_SKIP_INDEX_OPT_KEY_TYPE, +}; use snafu::ResultExt; use crate::error::{ @@ -119,3 +122,11 @@ pub fn validate_column_fulltext_create_option(key: &str) -> bool { ] .contains(&key) } + +pub fn validate_column_skip_index_create_option(key: &str) -> bool { + [ + COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIP_INDEX_OPT_KEY_TYPE, + ] + .contains(&key) +} diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index 25cc3bf7e5be..e44296385338 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -1519,6 +1519,7 @@ mod tests { .into(), ), vector_options: None, + skip_index_options: None, }, }; diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index e4ea46572e5f..83229f90983d 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -116,6 +116,8 @@ pub struct ColumnExtensions { pub fulltext_options: Option, /// Vector options. pub vector_options: Option, + /// Skip index options. + pub skip_index_options: Option, } impl Column { @@ -158,6 +160,15 @@ impl Display for Column { write!(f, " FULLTEXT")?; } } + + if let Some(skip_index_options) = &self.extensions.skip_index_options { + if !skip_index_options.is_empty() { + let options = skip_index_options.kv_pairs(); + write!(f, " SKIP WITH({})", format_list_comma!(options))?; + } else { + write!(f, " SKIP")?; + } + } Ok(()) } } From 3bed0dbbb0f98b0368d6faf5e7fdffa6d2e8e6ca Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Fri, 13 Dec 2024 11:24:16 +0800 Subject: [PATCH 2/6] wip: sqlness Signed-off-by: Ruihang Xia --- src/datatypes/src/error.rs | 9 +- src/datatypes/src/schema.rs | 2 +- src/datatypes/src/schema/column_schema.rs | 102 ++++++++++++++++++ src/sql/src/error.rs | 9 +- src/sql/src/statements.rs | 9 +- src/sql/src/statements/create.rs | 13 ++- .../create/create_with_skip_index.result | 33 ++++++ .../common/create/create_with_skip_index.sql | 14 +++ 8 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 tests/cases/standalone/common/create/create_with_skip_index.result create mode 100644 tests/cases/standalone/common/create/create_with_skip_index.sql diff --git a/src/datatypes/src/error.rs b/src/datatypes/src/error.rs index 705e5d9682cd..a7887e6801a7 100644 --- a/src/datatypes/src/error.rs +++ b/src/datatypes/src/error.rs @@ -232,6 +232,12 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + #[snafu(display("Invalid skip index option: {}", msg))] + InvalidSkipIndexOption { + msg: String, + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -252,7 +258,8 @@ impl ErrorExt for Error { | InvalidPrecisionOrScale { .. } | InvalidJson { .. } | InvalidVector { .. } - | InvalidFulltextOption { .. } => StatusCode::InvalidArguments, + | InvalidFulltextOption { .. } + | InvalidSkipIndexOption { .. } => StatusCode::InvalidArguments, ValueExceedsPrecision { .. } | CastType { .. } diff --git a/src/datatypes/src/schema.rs b/src/datatypes/src/schema.rs index d85f4371c1b6..05f540ffff82 100644 --- a/src/datatypes/src/schema.rs +++ b/src/datatypes/src/schema.rs @@ -28,7 +28,7 @@ use snafu::{ensure, ResultExt}; use crate::error::{self, DuplicateColumnSnafu, Error, ProjectArrowSchemaSnafu, Result}; use crate::prelude::ConcreteDataType; pub use crate::schema::column_schema::{ - ColumnSchema, FulltextAnalyzer, FulltextOptions, Metadata, + ColumnSchema, FulltextAnalyzer, FulltextOptions, Metadata, SkipIndexOptions, COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, COLUMN_SKIP_INDEX_OPT_KEY_TYPE, COMMENT_KEY, FULLTEXT_KEY, INVERTED_INDEX_KEY, TIME_INDEX_KEY, diff --git a/src/datatypes/src/schema/column_schema.rs b/src/datatypes/src/schema/column_schema.rs index aebbc862aaf0..1c6295f76ad6 100644 --- a/src/datatypes/src/schema/column_schema.rs +++ b/src/datatypes/src/schema/column_schema.rs @@ -39,6 +39,8 @@ const DEFAULT_CONSTRAINT_KEY: &str = "greptime:default_constraint"; pub const FULLTEXT_KEY: &str = "greptime:fulltext"; /// Key used to store whether the column has inverted index in arrow field's metadata. pub const INVERTED_INDEX_KEY: &str = "greptime:inverted_index"; +/// Key used to store skip options in arrow field's metadata. +pub const SKIP_KEY: &str = "greptime:skip"; /// Keys used in fulltext options pub const COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE: &str = "enable"; @@ -49,6 +51,8 @@ pub const COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE: &str = "case_sensitive"; pub const COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY: &str = "granularity"; pub const COLUMN_SKIP_INDEX_OPT_KEY_TYPE: &str = "type"; +pub const DEFAULT_GRANULARITY: u32 = 10240; + /// Schema of a column, used as an immutable struct. #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ColumnSchema { @@ -302,6 +306,34 @@ impl ColumnSchema { ); Ok(()) } + + /// Retrieves the skip options for the column. + pub fn skip_options(&self) -> Result> { + match self.metadata.get(SKIP_KEY) { + None => Ok(None), + Some(json) => { + let options = + serde_json::from_str(json).context(error::DeserializeSnafu { json })?; + Ok(Some(options)) + } + } + } + + pub fn with_skip_options(mut self, options: SkipIndexOptions) -> Result { + self.metadata.insert( + SKIP_KEY.to_string(), + serde_json::to_string(&options).context(error::SerializeSnafu)?, + ); + Ok(self) + } + + pub fn set_skip_options(&mut self, options: &SkipIndexOptions) -> Result<()> { + self.metadata.insert( + SKIP_KEY.to_string(), + serde_json::to_string(options).context(error::SerializeSnafu)?, + ); + Ok(()) + } } /// Column extended type set in column schema's metadata. @@ -499,6 +531,76 @@ impl fmt::Display for FulltextAnalyzer { } } +/// Skip options for a column. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, Visit, VisitMut)] +#[serde(rename_all = "kebab-case")] +pub struct SkipIndexOptions { + /// The granularity of the skip index. + pub granularity: u32, + /// The type of the skip index. + #[serde(default)] + pub index_type: SkipIndexType, +} + +impl fmt::Display for SkipIndexOptions { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "granularity={}", self.granularity)?; + write!(f, ", index_type={}", self.index_type)?; + Ok(()) + } +} + +/// Skip index types. +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, Visit, VisitMut)] +pub enum SkipIndexType { + #[default] + BloomFilter, +} + +impl fmt::Display for SkipIndexType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SkipIndexType::BloomFilter => write!(f, "BLOOM"), + } + } +} + +impl TryFrom> for SkipIndexOptions { + type Error = Error; + + fn try_from(options: HashMap) -> Result { + // Parse granularity with default value 1 + let granularity = match options.get(COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY) { + Some(value) => value.parse::().map_err(|_| { + error::InvalidSkipIndexOptionSnafu { + msg: format!("Invalid granularity: {value}, expected: positive integer"), + } + .build() + })?, + None => DEFAULT_GRANULARITY, + }; + + // Parse index type with default value BloomFilter + let index_type = match options.get(COLUMN_SKIP_INDEX_OPT_KEY_TYPE) { + Some(typ) => match typ.to_ascii_uppercase().as_str() { + "BLOOM" => SkipIndexType::BloomFilter, + _ => { + return error::InvalidSkipIndexOptionSnafu { + msg: format!("Invalid index type: {typ}, expected: 'BLOOM'"), + } + .fail(); + } + }, + None => SkipIndexType::default(), + }; + + Ok(SkipIndexOptions { + granularity, + index_type, + }) + } +} + #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/src/sql/src/error.rs b/src/sql/src/error.rs index d05ccf8e5470..15cb0e06a128 100644 --- a/src/sql/src/error.rs +++ b/src/sql/src/error.rs @@ -326,6 +326,13 @@ pub enum Error { location: Location, }, + #[snafu(display("Failed to set skip index option"))] + SetSkipIndexOption { + source: datatypes::error::Error, + #[snafu(implicit)] + location: Location, + }, + #[snafu(display("Datatype error: {}", source))] Datatype { source: datatypes::error::Error, @@ -375,7 +382,7 @@ impl ErrorExt for Error { ConvertSqlValue { .. } | ConvertValue { .. } => StatusCode::Unsupported, PermissionDenied { .. } => StatusCode::PermissionDenied, - SetFulltextOption { .. } => StatusCode::Unexpected, + SetFulltextOption { .. } | SetSkipIndexOption { .. } => StatusCode::Unexpected, } } diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index e44296385338..a17c4b6d03ed 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -58,7 +58,8 @@ use crate::error::{ self, ColumnTypeMismatchSnafu, ConvertSqlValueSnafu, ConvertToGrpcDataTypeSnafu, ConvertValueSnafu, DatatypeSnafu, InvalidCastSnafu, InvalidSqlValueSnafu, InvalidUnaryOpSnafu, ParseSqlValueSnafu, Result, SerializeColumnDefaultConstraintSnafu, SetFulltextOptionSnafu, - TimestampOverflowSnafu, UnsupportedDefaultValueSnafu, UnsupportedUnaryOpSnafu, + SetSkipIndexOptionSnafu, TimestampOverflowSnafu, UnsupportedDefaultValueSnafu, + UnsupportedUnaryOpSnafu, }; use crate::statements::create::Column; pub use crate::statements::option_map::OptionMap; @@ -513,6 +514,12 @@ pub fn column_to_schema( .context(SetFulltextOptionSnafu)?; } + if let Some(options) = column.extensions.build_skip_index_options()? { + column_schema = column_schema + .with_skip_options(options) + .context(SetSkipIndexOptionSnafu)?; + } + Ok(column_schema) } diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 83229f90983d..29234dcf2afc 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use std::fmt::{Display, Formatter}; use common_catalog::consts::FILE_ENGINE; -use datatypes::schema::FulltextOptions; +use datatypes::schema::{FulltextOptions, SkipIndexOptions}; use itertools::Itertools; use serde::Serialize; use snafu::ResultExt; @@ -24,7 +24,7 @@ use sqlparser::ast::{ColumnOptionDef, DataType, Expr, Query}; use sqlparser_derive::{Visit, VisitMut}; use crate::ast::{ColumnDef, Ident, ObjectName, Value as SqlValue}; -use crate::error::{Result, SetFulltextOptionSnafu}; +use crate::error::{Result, SetFulltextOptionSnafu, SetSkipIndexOptionSnafu}; use crate::statements::statement::Statement; use crate::statements::OptionMap; @@ -182,6 +182,15 @@ impl ColumnExtensions { let options: HashMap = options.clone().into_map(); Ok(Some(options.try_into().context(SetFulltextOptionSnafu)?)) } + + pub fn build_skip_index_options(&self) -> Result> { + let Some(options) = self.skip_index_options.as_ref() else { + return Ok(None); + }; + + let options: HashMap = options.clone().into_map(); + Ok(Some(options.try_into().context(SetSkipIndexOptionSnafu)?)) + } } #[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut, Serialize)] diff --git a/tests/cases/standalone/common/create/create_with_skip_index.result b/tests/cases/standalone/common/create/create_with_skip_index.result new file mode 100644 index 000000000000..4a5c32ad5a89 --- /dev/null +++ b/tests/cases/standalone/common/create/create_with_skip_index.result @@ -0,0 +1,33 @@ +create table + skipping_table ( + ts timestamp time index, + id string skip, + `name` string skip + with + (granularity = 8192), + ); + +Affected Rows: 0 + +show +create table + skipping_table; + ++----------------+-----------------------------------------------+ +| Table | Create Table | ++----------------+-----------------------------------------------+ +| skipping_table | CREATE TABLE IF NOT EXISTS "skipping_table" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "id" STRING NULL, | +| | "name" STRING NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------------+-----------------------------------------------+ + +drop table skipping_table; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/create/create_with_skip_index.sql b/tests/cases/standalone/common/create/create_with_skip_index.sql new file mode 100644 index 000000000000..f74a74c8b060 --- /dev/null +++ b/tests/cases/standalone/common/create/create_with_skip_index.sql @@ -0,0 +1,14 @@ +create table + skipping_table ( + ts timestamp time index, + id string skip, + `name` string skip + with + (granularity = 8192), + ); + +show +create table + skipping_table; + +drop table skipping_table; \ No newline at end of file From 173b9a8035c8d8a4b3554f5f4c6d9b46d14f021c Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Fri, 13 Dec 2024 15:34:32 +0800 Subject: [PATCH 3/6] impl show create part Signed-off-by: Ruihang Xia --- src/api/src/v1/column_def.rs | 12 ++++++- src/datatypes/src/schema.rs | 3 +- src/operator/src/statement/ddl.rs | 3 +- src/query/src/error.rs | 11 +++++- src/query/src/sql/show_create_table.rs | 34 ++++++++++++++++--- .../create/create_with_skip_index.result | 26 +++++++------- 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/api/src/v1/column_def.rs b/src/api/src/v1/column_def.rs index f026d3f6f97f..9e4d31cd0d29 100644 --- a/src/api/src/v1/column_def.rs +++ b/src/api/src/v1/column_def.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use datatypes::schema::{ ColumnDefaultConstraint, ColumnSchema, FulltextAnalyzer, FulltextOptions, COMMENT_KEY, - FULLTEXT_KEY, INVERTED_INDEX_KEY, + FULLTEXT_KEY, INVERTED_INDEX_KEY, SKIP_KEY, }; use greptime_proto::v1::Analyzer; use snafu::ResultExt; @@ -29,6 +29,8 @@ use crate::v1::{ColumnDef, ColumnOptions, SemanticType}; const FULLTEXT_GRPC_KEY: &str = "fulltext"; /// Key used to store inverted index options in gRPC column options. const INVERTED_INDEX_GRPC_KEY: &str = "inverted_index"; +/// Key used to store skip index options in gRPC column options. +const SKIP_INDEX_GRPC_KEY: &str = "skip_index"; /// Tries to construct a `ColumnSchema` from the given `ColumnDef`. pub fn try_as_column_schema(column_def: &ColumnDef) -> Result { @@ -60,6 +62,9 @@ pub fn try_as_column_schema(column_def: &ColumnDef) -> Result { if let Some(inverted_index) = options.options.get(INVERTED_INDEX_GRPC_KEY) { metadata.insert(INVERTED_INDEX_KEY.to_string(), inverted_index.clone()); } + if let Some(skip_index) = options.options.get(SKIP_INDEX_GRPC_KEY) { + metadata.insert(SKIP_KEY.to_string(), skip_index.clone()); + } } ColumnSchema::new(&column_def.name, data_type.into(), column_def.is_nullable) @@ -84,6 +89,11 @@ pub fn options_from_column_schema(column_schema: &ColumnSchema) -> Option = + Arc::new(table_info.try_into().context(CreateTableInfoSnafu)?); create_table.table_id = Some(api::v1::TableId { id: table_id }); let table = DistTable::table(table_info); diff --git a/src/query/src/error.rs b/src/query/src/error.rs index 7e246d11c332..cf3d7e097112 100644 --- a/src/query/src/error.rs +++ b/src/query/src/error.rs @@ -316,6 +316,13 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(display("Failed to get skip inde options"))] + GetSkipIndexOptions { + source: datatypes::error::Error, + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -366,7 +373,9 @@ impl ErrorExt for Error { MissingTableMutationHandler { .. } => StatusCode::Unexpected, GetRegionMetadata { .. } => StatusCode::RegionNotReady, TableReadOnly { .. } => StatusCode::Unsupported, - GetFulltextOptions { source, .. } => source.status_code(), + GetFulltextOptions { source, .. } | GetSkipIndexOptions { source, .. } => { + source.status_code() + } } } diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index ca69dfc5e69e..47895e50117c 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -19,7 +19,8 @@ use std::collections::HashMap; use common_meta::SchemaOptions; use datatypes::schema::{ ColumnDefaultConstraint, ColumnSchema, SchemaRef, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, - COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COMMENT_KEY, + COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIP_INDEX_OPT_KEY_TYPE, COMMENT_KEY, }; use snafu::ResultExt; use sql::ast::{ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, ObjectName}; @@ -32,7 +33,8 @@ use table::metadata::{TableInfoRef, TableMeta}; use table::requests::{FILE_TABLE_META_KEY, TTL_KEY, WRITE_BUFFER_SIZE_KEY}; use crate::error::{ - ConvertSqlTypeSnafu, ConvertSqlValueSnafu, GetFulltextOptionsSnafu, Result, SqlSnafu, + ConvertSqlTypeSnafu, ConvertSqlValueSnafu, GetFulltextOptionsSnafu, GetSkipIndexOptionsSnafu, + Result, SqlSnafu, }; /// Generates CREATE TABLE options from given table metadata and schema-level options. @@ -115,6 +117,23 @@ fn create_column(column_schema: &ColumnSchema, quote_style: char) -> Result Date: Fri, 13 Dec 2024 15:47:29 +0800 Subject: [PATCH 4/6] add empty line Signed-off-by: Ruihang Xia --- tests/cases/standalone/common/create/create_with_skip_index.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/standalone/common/create/create_with_skip_index.sql b/tests/cases/standalone/common/create/create_with_skip_index.sql index f74a74c8b060..df481ad185b7 100644 --- a/tests/cases/standalone/common/create/create_with_skip_index.sql +++ b/tests/cases/standalone/common/create/create_with_skip_index.sql @@ -11,4 +11,4 @@ show create table skipping_table; -drop table skipping_table; \ No newline at end of file +drop table skipping_table; From 8315e74324d387f6bcdd8ebf76ff7a09ea5c1d7e Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Mon, 16 Dec 2024 15:39:40 +0800 Subject: [PATCH 5/6] change keyword to SKIPPING INDEX Signed-off-by: Ruihang Xia --- src/api/src/v1/column_def.rs | 12 +++--- src/datatypes/src/error.rs | 6 +-- src/datatypes/src/schema.rs | 8 ++-- src/datatypes/src/schema/column_schema.rs | 40 +++++++++---------- src/query/src/error.rs | 6 +-- src/query/src/sql/show_create_table.rs | 24 +++++------ src/sql/src/error.rs | 6 +-- src/sql/src/parsers/create_parser.rs | 28 +++++++++---- src/sql/src/parsers/utils.rs | 6 +-- src/sql/src/statements.rs | 10 ++--- src/sql/src/statements/create.rs | 22 +++++----- .../create/create_with_skip_index.result | 30 +++++++------- .../common/create/create_with_skip_index.sql | 4 +- 13 files changed, 108 insertions(+), 94 deletions(-) diff --git a/src/api/src/v1/column_def.rs b/src/api/src/v1/column_def.rs index 9e4d31cd0d29..68d4e6e0c1d1 100644 --- a/src/api/src/v1/column_def.rs +++ b/src/api/src/v1/column_def.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use datatypes::schema::{ ColumnDefaultConstraint, ColumnSchema, FulltextAnalyzer, FulltextOptions, COMMENT_KEY, - FULLTEXT_KEY, INVERTED_INDEX_KEY, SKIP_KEY, + FULLTEXT_KEY, INVERTED_INDEX_KEY, SKIPPING_INDEX_KEY, }; use greptime_proto::v1::Analyzer; use snafu::ResultExt; @@ -30,7 +30,7 @@ const FULLTEXT_GRPC_KEY: &str = "fulltext"; /// Key used to store inverted index options in gRPC column options. const INVERTED_INDEX_GRPC_KEY: &str = "inverted_index"; /// Key used to store skip index options in gRPC column options. -const SKIP_INDEX_GRPC_KEY: &str = "skip_index"; +const SKIPPING_INDEX_GRPC_KEY: &str = "skipping_index"; /// Tries to construct a `ColumnSchema` from the given `ColumnDef`. pub fn try_as_column_schema(column_def: &ColumnDef) -> Result { @@ -62,8 +62,8 @@ pub fn try_as_column_schema(column_def: &ColumnDef) -> Result { if let Some(inverted_index) = options.options.get(INVERTED_INDEX_GRPC_KEY) { metadata.insert(INVERTED_INDEX_KEY.to_string(), inverted_index.clone()); } - if let Some(skip_index) = options.options.get(SKIP_INDEX_GRPC_KEY) { - metadata.insert(SKIP_KEY.to_string(), skip_index.clone()); + if let Some(skip_index) = options.options.get(SKIPPING_INDEX_GRPC_KEY) { + metadata.insert(SKIPPING_INDEX_KEY.to_string(), skip_index.clone()); } } @@ -89,10 +89,10 @@ pub fn options_from_column_schema(column_schema: &ColumnSchema) -> Option StatusCode::InvalidArguments, + | InvalidSkippingIndexOption { .. } => StatusCode::InvalidArguments, ValueExceedsPrecision { .. } | CastType { .. } diff --git a/src/datatypes/src/schema.rs b/src/datatypes/src/schema.rs index fa4e8b53c8d4..c537a4608b42 100644 --- a/src/datatypes/src/schema.rs +++ b/src/datatypes/src/schema.rs @@ -28,11 +28,11 @@ use snafu::{ensure, ResultExt}; use crate::error::{self, DuplicateColumnSnafu, Error, ProjectArrowSchemaSnafu, Result}; use crate::prelude::ConcreteDataType; pub use crate::schema::column_schema::{ - ColumnSchema, FulltextAnalyzer, FulltextOptions, Metadata, SkipIndexOptions, + ColumnSchema, FulltextAnalyzer, FulltextOptions, Metadata, SkippingIndexOptions, COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, - COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, - COLUMN_SKIP_INDEX_OPT_KEY_TYPE, COMMENT_KEY, FULLTEXT_KEY, INVERTED_INDEX_KEY, SKIP_KEY, - TIME_INDEX_KEY, + COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE, COMMENT_KEY, FULLTEXT_KEY, INVERTED_INDEX_KEY, + SKIPPING_INDEX_KEY, TIME_INDEX_KEY, }; pub use crate::schema::constraint::ColumnDefaultConstraint; pub use crate::schema::raw::RawSchema; diff --git a/src/datatypes/src/schema/column_schema.rs b/src/datatypes/src/schema/column_schema.rs index 1c6295f76ad6..aee9efd9625d 100644 --- a/src/datatypes/src/schema/column_schema.rs +++ b/src/datatypes/src/schema/column_schema.rs @@ -40,16 +40,16 @@ pub const FULLTEXT_KEY: &str = "greptime:fulltext"; /// Key used to store whether the column has inverted index in arrow field's metadata. pub const INVERTED_INDEX_KEY: &str = "greptime:inverted_index"; /// Key used to store skip options in arrow field's metadata. -pub const SKIP_KEY: &str = "greptime:skip"; +pub const SKIPPING_INDEX_KEY: &str = "greptime:skipping_index"; /// Keys used in fulltext options pub const COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE: &str = "enable"; pub const COLUMN_FULLTEXT_OPT_KEY_ANALYZER: &str = "analyzer"; pub const COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE: &str = "case_sensitive"; -/// Keys used in SKIP index options -pub const COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY: &str = "granularity"; -pub const COLUMN_SKIP_INDEX_OPT_KEY_TYPE: &str = "type"; +/// Keys used in SKIPPING index options +pub const COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY: &str = "granularity"; +pub const COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE: &str = "type"; pub const DEFAULT_GRANULARITY: u32 = 10240; @@ -307,9 +307,9 @@ impl ColumnSchema { Ok(()) } - /// Retrieves the skip options for the column. - pub fn skip_options(&self) -> Result> { - match self.metadata.get(SKIP_KEY) { + /// Retrieves the skipping index options for the column. + pub fn skipping_index_options(&self) -> Result> { + match self.metadata.get(SKIPPING_INDEX_KEY) { None => Ok(None), Some(json) => { let options = @@ -319,17 +319,17 @@ impl ColumnSchema { } } - pub fn with_skip_options(mut self, options: SkipIndexOptions) -> Result { + pub fn with_skipping_options(mut self, options: SkippingIndexOptions) -> Result { self.metadata.insert( - SKIP_KEY.to_string(), + SKIPPING_INDEX_KEY.to_string(), serde_json::to_string(&options).context(error::SerializeSnafu)?, ); Ok(self) } - pub fn set_skip_options(&mut self, options: &SkipIndexOptions) -> Result<()> { + pub fn set_skipping_options(&mut self, options: &SkippingIndexOptions) -> Result<()> { self.metadata.insert( - SKIP_KEY.to_string(), + SKIPPING_INDEX_KEY.to_string(), serde_json::to_string(options).context(error::SerializeSnafu)?, ); Ok(()) @@ -531,10 +531,10 @@ impl fmt::Display for FulltextAnalyzer { } } -/// Skip options for a column. +/// Skipping options for a column. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, Visit, VisitMut)] #[serde(rename_all = "kebab-case")] -pub struct SkipIndexOptions { +pub struct SkippingIndexOptions { /// The granularity of the skip index. pub granularity: u32, /// The type of the skip index. @@ -542,7 +542,7 @@ pub struct SkipIndexOptions { pub index_type: SkipIndexType, } -impl fmt::Display for SkipIndexOptions { +impl fmt::Display for SkippingIndexOptions { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "granularity={}", self.granularity)?; write!(f, ", index_type={}", self.index_type)?; @@ -565,14 +565,14 @@ impl fmt::Display for SkipIndexType { } } -impl TryFrom> for SkipIndexOptions { +impl TryFrom> for SkippingIndexOptions { type Error = Error; fn try_from(options: HashMap) -> Result { // Parse granularity with default value 1 - let granularity = match options.get(COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY) { + let granularity = match options.get(COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY) { Some(value) => value.parse::().map_err(|_| { - error::InvalidSkipIndexOptionSnafu { + error::InvalidSkippingIndexOptionSnafu { msg: format!("Invalid granularity: {value}, expected: positive integer"), } .build() @@ -581,11 +581,11 @@ impl TryFrom> for SkipIndexOptions { }; // Parse index type with default value BloomFilter - let index_type = match options.get(COLUMN_SKIP_INDEX_OPT_KEY_TYPE) { + let index_type = match options.get(COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE) { Some(typ) => match typ.to_ascii_uppercase().as_str() { "BLOOM" => SkipIndexType::BloomFilter, _ => { - return error::InvalidSkipIndexOptionSnafu { + return error::InvalidSkippingIndexOptionSnafu { msg: format!("Invalid index type: {typ}, expected: 'BLOOM'"), } .fail(); @@ -594,7 +594,7 @@ impl TryFrom> for SkipIndexOptions { None => SkipIndexType::default(), }; - Ok(SkipIndexOptions { + Ok(SkippingIndexOptions { granularity, index_type, }) diff --git a/src/query/src/error.rs b/src/query/src/error.rs index cf3d7e097112..e696008cf546 100644 --- a/src/query/src/error.rs +++ b/src/query/src/error.rs @@ -317,8 +317,8 @@ pub enum Error { location: Location, }, - #[snafu(display("Failed to get skip inde options"))] - GetSkipIndexOptions { + #[snafu(display("Failed to get SKIPPING index options"))] + GetSkippingIndexOptions { source: datatypes::error::Error, #[snafu(implicit)] location: Location, @@ -373,7 +373,7 @@ impl ErrorExt for Error { MissingTableMutationHandler { .. } => StatusCode::Unexpected, GetRegionMetadata { .. } => StatusCode::RegionNotReady, TableReadOnly { .. } => StatusCode::Unsupported, - GetFulltextOptions { source, .. } | GetSkipIndexOptions { source, .. } => { + GetFulltextOptions { source, .. } | GetSkippingIndexOptions { source, .. } => { source.status_code() } } diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index 47895e50117c..b903509d2270 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -19,8 +19,8 @@ use std::collections::HashMap; use common_meta::SchemaOptions; use datatypes::schema::{ ColumnDefaultConstraint, ColumnSchema, SchemaRef, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, - COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, - COLUMN_SKIP_INDEX_OPT_KEY_TYPE, COMMENT_KEY, + COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE, COMMENT_KEY, }; use snafu::ResultExt; use sql::ast::{ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, ObjectName}; @@ -33,8 +33,8 @@ use table::metadata::{TableInfoRef, TableMeta}; use table::requests::{FILE_TABLE_META_KEY, TTL_KEY, WRITE_BUFFER_SIZE_KEY}; use crate::error::{ - ConvertSqlTypeSnafu, ConvertSqlValueSnafu, GetFulltextOptionsSnafu, GetSkipIndexOptionsSnafu, - Result, SqlSnafu, + ConvertSqlTypeSnafu, ConvertSqlValueSnafu, GetFulltextOptionsSnafu, + GetSkippingIndexOptionsSnafu, Result, SqlSnafu, }; /// Generates CREATE TABLE options from given table metadata and schema-level options. @@ -118,20 +118,20 @@ fn create_column(column_schema: &ColumnSchema, quote_style: char) -> Result StatusCode::Unsupported, PermissionDenied { .. } => StatusCode::PermissionDenied, - SetFulltextOption { .. } | SetSkipIndexOption { .. } => StatusCode::Unexpected, + SetFulltextOption { .. } | SetSkippingIndexOption { .. } => StatusCode::Unexpected, } } diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 83ffa0e502d4..25db189fb3af 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -55,6 +55,7 @@ pub const SINK: &str = "SINK"; pub const EXPIRE: &str = "EXPIRE"; pub const AFTER: &str = "AFTER"; pub const INVERTED: &str = "INVERTED"; +pub const SKIPPING: &str = "SKIPPING"; const DB_OPT_KEY_TTL: &str = "ttl"; @@ -705,12 +706,23 @@ impl<'a> ParserContext<'a> { let mut is_index_declared = false; - if parser.parse_keyword(Keyword::SKIP) { + if let Token::Word(word) = parser.peek_token().token + && word.value.eq_ignore_ascii_case(SKIPPING) + { + parser.next_token(); + // Consume `INDEX` keyword + ensure!( + parser.parse_keyword(Keyword::INDEX), + InvalidColumnOptionSnafu { + name: column_name.to_string(), + msg: "expect INDEX after SKIPPING keyword", + } + ); ensure!( - column_extensions.skip_index_options.is_none(), + column_extensions.skipping_index_options.is_none(), InvalidColumnOptionSnafu { name: column_name.to_string(), - msg: "duplicated SKIP index option", + msg: "duplicated SKIPPING index option", } ); @@ -731,7 +743,7 @@ impl<'a> ParserContext<'a> { ); } - column_extensions.skip_index_options = Some(options.into()); + column_extensions.skipping_index_options = Some(options.into()); is_index_declared |= true; } @@ -2142,7 +2154,7 @@ CREATE TABLE log ( let sql = r" CREATE TABLE log ( ts TIMESTAMP TIME INDEX, - msg INT SKIP WITH (granularity='8192', type='bloom'), + msg INT SKIPPING INDEX WITH (granularity='8192', type='bloom'), )"; let result = ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) @@ -2153,7 +2165,7 @@ CREATE TABLE log ( if col.name().value == "msg" { assert!(!col .extensions - .skip_index_options + .skipping_index_options .as_ref() .unwrap() .is_empty()); @@ -2166,7 +2178,7 @@ CREATE TABLE log ( let sql = r" CREATE TABLE log ( ts TIMESTAMP TIME INDEX, - msg INT SKIP, + msg INT SKIPPING INDEX, )"; let result = ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) @@ -2177,7 +2189,7 @@ CREATE TABLE log ( if col.name().value == "msg" { assert!(col .extensions - .skip_index_options + .skipping_index_options .as_ref() .unwrap() .is_empty()); diff --git a/src/sql/src/parsers/utils.rs b/src/sql/src/parsers/utils.rs index ef281b50ac38..af4eae40afaf 100644 --- a/src/sql/src/parsers/utils.rs +++ b/src/sql/src/parsers/utils.rs @@ -28,7 +28,7 @@ use datafusion_sql::TableReference; use datatypes::arrow::datatypes::DataType; use datatypes::schema::{ COLUMN_FULLTEXT_OPT_KEY_ANALYZER, COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, - COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, COLUMN_SKIP_INDEX_OPT_KEY_TYPE, + COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY, COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE, }; use snafu::ResultExt; @@ -125,8 +125,8 @@ pub fn validate_column_fulltext_create_option(key: &str) -> bool { pub fn validate_column_skip_index_create_option(key: &str) -> bool { [ - COLUMN_SKIP_INDEX_OPT_KEY_GRANULARITY, - COLUMN_SKIP_INDEX_OPT_KEY_TYPE, + COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY, + COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE, ] .contains(&key) } diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index a17c4b6d03ed..00196ed5313b 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -58,7 +58,7 @@ use crate::error::{ self, ColumnTypeMismatchSnafu, ConvertSqlValueSnafu, ConvertToGrpcDataTypeSnafu, ConvertValueSnafu, DatatypeSnafu, InvalidCastSnafu, InvalidSqlValueSnafu, InvalidUnaryOpSnafu, ParseSqlValueSnafu, Result, SerializeColumnDefaultConstraintSnafu, SetFulltextOptionSnafu, - SetSkipIndexOptionSnafu, TimestampOverflowSnafu, UnsupportedDefaultValueSnafu, + SetSkippingIndexOptionSnafu, TimestampOverflowSnafu, UnsupportedDefaultValueSnafu, UnsupportedUnaryOpSnafu, }; use crate::statements::create::Column; @@ -514,10 +514,10 @@ pub fn column_to_schema( .context(SetFulltextOptionSnafu)?; } - if let Some(options) = column.extensions.build_skip_index_options()? { + if let Some(options) = column.extensions.build_skipping_index_options()? { column_schema = column_schema - .with_skip_options(options) - .context(SetSkipIndexOptionSnafu)?; + .with_skipping_options(options) + .context(SetSkippingIndexOptionSnafu)?; } Ok(column_schema) @@ -1526,7 +1526,7 @@ mod tests { .into(), ), vector_options: None, - skip_index_options: None, + skipping_index_options: None, }, }; diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 29234dcf2afc..f6ca2edcb9a5 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use std::fmt::{Display, Formatter}; use common_catalog::consts::FILE_ENGINE; -use datatypes::schema::{FulltextOptions, SkipIndexOptions}; +use datatypes::schema::{FulltextOptions, SkippingIndexOptions}; use itertools::Itertools; use serde::Serialize; use snafu::ResultExt; @@ -24,7 +24,7 @@ use sqlparser::ast::{ColumnOptionDef, DataType, Expr, Query}; use sqlparser_derive::{Visit, VisitMut}; use crate::ast::{ColumnDef, Ident, ObjectName, Value as SqlValue}; -use crate::error::{Result, SetFulltextOptionSnafu, SetSkipIndexOptionSnafu}; +use crate::error::{Result, SetFulltextOptionSnafu, SetSkippingIndexOptionSnafu}; use crate::statements::statement::Statement; use crate::statements::OptionMap; @@ -116,8 +116,8 @@ pub struct ColumnExtensions { pub fulltext_options: Option, /// Vector options. pub vector_options: Option, - /// Skip index options. - pub skip_index_options: Option, + /// Skipping index options. + pub skipping_index_options: Option, } impl Column { @@ -161,12 +161,12 @@ impl Display for Column { } } - if let Some(skip_index_options) = &self.extensions.skip_index_options { + if let Some(skip_index_options) = &self.extensions.skipping_index_options { if !skip_index_options.is_empty() { let options = skip_index_options.kv_pairs(); - write!(f, " SKIP WITH({})", format_list_comma!(options))?; + write!(f, " SKIPPING INDEX WITH({})", format_list_comma!(options))?; } else { - write!(f, " SKIP")?; + write!(f, " SKIPPING INDEX")?; } } Ok(()) @@ -183,13 +183,15 @@ impl ColumnExtensions { Ok(Some(options.try_into().context(SetFulltextOptionSnafu)?)) } - pub fn build_skip_index_options(&self) -> Result> { - let Some(options) = self.skip_index_options.as_ref() else { + pub fn build_skipping_index_options(&self) -> Result> { + let Some(options) = self.skipping_index_options.as_ref() else { return Ok(None); }; let options: HashMap = options.clone().into_map(); - Ok(Some(options.try_into().context(SetSkipIndexOptionSnafu)?)) + Ok(Some( + options.try_into().context(SetSkippingIndexOptionSnafu)?, + )) } } diff --git a/tests/cases/standalone/common/create/create_with_skip_index.result b/tests/cases/standalone/common/create/create_with_skip_index.result index 0f6810536262..00dd24dc6c9a 100644 --- a/tests/cases/standalone/common/create/create_with_skip_index.result +++ b/tests/cases/standalone/common/create/create_with_skip_index.result @@ -1,8 +1,8 @@ create table skipping_table ( ts timestamp time index, - id string skip, - `name` string skip + id string skipping index, + `name` string skipping index with (granularity = 8192), ); @@ -13,19 +13,19 @@ show create table skipping_table; -+----------------+-----------------------------------------------------------------------+ -| Table | Create Table | -+----------------+-----------------------------------------------------------------------+ -| skipping_table | CREATE TABLE IF NOT EXISTS "skipping_table" ( | -| | "ts" TIMESTAMP(3) NOT NULL, | -| | "id" STRING NULL SKIP WITH(granularity = '10240', type = 'BLOOM'), | -| | "name" STRING NULL SKIP WITH(granularity = '8192', type = 'BLOOM'), | -| | TIME INDEX ("ts") | -| | ) | -| | | -| | ENGINE=mito | -| | | -+----------------+-----------------------------------------------------------------------+ ++----------------+---------------------------------------------------------------------------------+ +| Table | Create Table | ++----------------+---------------------------------------------------------------------------------+ +| skipping_table | CREATE TABLE IF NOT EXISTS "skipping_table" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "id" STRING NULL SKIPPING INDEX WITH(granularity = '10240', type = 'BLOOM'), | +| | "name" STRING NULL SKIPPING INDEX WITH(granularity = '8192', type = 'BLOOM'), | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------------+---------------------------------------------------------------------------------+ drop table skipping_table; diff --git a/tests/cases/standalone/common/create/create_with_skip_index.sql b/tests/cases/standalone/common/create/create_with_skip_index.sql index df481ad185b7..0558936699a4 100644 --- a/tests/cases/standalone/common/create/create_with_skip_index.sql +++ b/tests/cases/standalone/common/create/create_with_skip_index.sql @@ -1,8 +1,8 @@ create table skipping_table ( ts timestamp time index, - id string skip, - `name` string skip + id string skipping index, + `name` string skipping index with (granularity = 8192), ); From 2aa0d3b353fc38eeb37e7f1608be5dc8eb332046 Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Mon, 16 Dec 2024 17:00:19 +0800 Subject: [PATCH 6/6] rename local variables Signed-off-by: Ruihang Xia --- src/api/src/v1/column_def.rs | 8 ++++---- src/sql/src/parsers/create_parser.rs | 4 ++-- src/sql/src/parsers/utils.rs | 2 +- src/sql/src/statements/create.rs | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/api/src/v1/column_def.rs b/src/api/src/v1/column_def.rs index 68d4e6e0c1d1..77dcd2c62190 100644 --- a/src/api/src/v1/column_def.rs +++ b/src/api/src/v1/column_def.rs @@ -62,8 +62,8 @@ pub fn try_as_column_schema(column_def: &ColumnDef) -> Result { if let Some(inverted_index) = options.options.get(INVERTED_INDEX_GRPC_KEY) { metadata.insert(INVERTED_INDEX_KEY.to_string(), inverted_index.clone()); } - if let Some(skip_index) = options.options.get(SKIPPING_INDEX_GRPC_KEY) { - metadata.insert(SKIPPING_INDEX_KEY.to_string(), skip_index.clone()); + if let Some(skipping_index) = options.options.get(SKIPPING_INDEX_GRPC_KEY) { + metadata.insert(SKIPPING_INDEX_KEY.to_string(), skipping_index.clone()); } } @@ -89,10 +89,10 @@ pub fn options_from_column_schema(column_schema: &ColumnSchema) -> Option ParserContext<'a> { for key in options.keys() { ensure!( - validate_column_skip_index_create_option(key), + validate_column_skipping_index_create_option(key), InvalidColumnOptionSnafu { name: column_name.to_string(), msg: format!("invalid SKIP option: {key}"), diff --git a/src/sql/src/parsers/utils.rs b/src/sql/src/parsers/utils.rs index af4eae40afaf..f7eefc4b9562 100644 --- a/src/sql/src/parsers/utils.rs +++ b/src/sql/src/parsers/utils.rs @@ -123,7 +123,7 @@ pub fn validate_column_fulltext_create_option(key: &str) -> bool { .contains(&key) } -pub fn validate_column_skip_index_create_option(key: &str) -> bool { +pub fn validate_column_skipping_index_create_option(key: &str) -> bool { [ COLUMN_SKIPPING_INDEX_OPT_KEY_GRANULARITY, COLUMN_SKIPPING_INDEX_OPT_KEY_TYPE, diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index f6ca2edcb9a5..3ea265fb7f40 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -161,9 +161,9 @@ impl Display for Column { } } - if let Some(skip_index_options) = &self.extensions.skipping_index_options { - if !skip_index_options.is_empty() { - let options = skip_index_options.kv_pairs(); + if let Some(skipping_index_options) = &self.extensions.skipping_index_options { + if !skipping_index_options.is_empty() { + let options = skipping_index_options.kv_pairs(); write!(f, " SKIPPING INDEX WITH({})", format_list_comma!(options))?; } else { write!(f, " SKIPPING INDEX")?;