Skip to content

Commit

Permalink
feat: Alter column option
Browse files Browse the repository at this point in the history
  • Loading branch information
irenjj committed Aug 28, 2024
1 parent c8de8b8 commit 693130c
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ etcd-client = { version = "0.13" }
fst = "0.4.7"
futures = "0.3"
futures-util = "0.3"
greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "c437b55725b7f5224fe9d46db21072b4a682ee4b" }
greptime-proto = { git = "https://github.com/irenjj/greptime-proto.git", rev = "ca5920ee9263650c37556edad4bc877a674af444" }
humantime = "2.1"
humantime-serde = "1.1"
itertools = "0.10"
Expand Down Expand Up @@ -151,7 +151,7 @@ reqwest = { version = "0.12", default-features = false, features = [
"stream",
"multipart",
] }
# SCRAM-SHA-512 requires https://github.com/dequbed/rsasl/pull/48, https://github.com/influxdata/rskafka/pull/247
# SCRAM-SHA-512 requires https://github.com/dequbed/rsasl/pull/48, https://github.com/influxdata/rskafka/pull/247
rskafka = { git = "https://github.com/WenyXu/rskafka.git", rev = "940c6030012c5b746fad819fb72e3325b26e39de", features = [
"transport-tls",
] }
Expand Down
1 change: 1 addition & 0 deletions src/common/grpc-expr/src/alter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn alter_expr_to_request(table_id: TableId, expr: AlterExpr) -> Result<Alter
Kind::RenameTable(RenameTable { new_table_name }) => {
AlterKind::RenameTable { new_table_name }
}
Kind::AddFulltext(_) => { todo!() },
};

let request = AlterTableRequest {
Expand Down
1 change: 1 addition & 0 deletions src/common/meta/src/ddl/alter_table/region_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ fn create_proto_alter_kind(
})))
}
Kind::RenameTable(_) => Ok(None),
Kind::AddFulltext(_) => todo!(),
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/common/meta/src/ddl/alter_table/update_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ impl AlterTableProcedure {
new_info.name = new_table_name.to_string();
}
AlterKind::DropColumns { .. } | AlterKind::ChangeColumnTypes { .. } => {}
AlterKind::AddFulltexts { column_name, options } => {
},
}

Ok(new_info)
Expand Down
13 changes: 10 additions & 3 deletions src/operator/src/expr_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ use api::helper::ColumnDataTypeWrapper;
use api::v1::alter_expr::Kind;
use api::v1::column_def::options_from_column_schema;
use api::v1::{
AddColumn, AddColumns, AlterExpr, ChangeColumnType, ChangeColumnTypes, ColumnDataType,
ColumnDataTypeExtension, CreateFlowExpr, CreateTableExpr, CreateViewExpr, DropColumn,
DropColumns, ExpireAfter, RenameTable, SemanticType, TableName,
AddColumn, AddColumns, AddFulltext, AlterExpr, ChangeColumnType, ChangeColumnTypes,
ColumnDataType, ColumnDataTypeExtension, CreateFlowExpr, CreateTableExpr, CreateViewExpr,
DropColumn, DropColumns, ExpireAfter, RenameTable, SemanticType, TableName,
};
use common_error::ext::BoxedError;
use common_grpc_expr::util::ColumnExpr;
Expand Down Expand Up @@ -483,6 +483,13 @@ pub(crate) fn to_alter_expr(
AlterTableOperation::RenameTable { new_table_name } => Kind::RenameTable(RenameTable {
new_table_name: new_table_name.to_string(),
}),
AlterTableOperation::AlterColumnFulltext {
column_name,
options,
} => Kind::AddFulltext(AddFulltext {
column_name: column_name.value.to_string(),
options: options.hash_options.clone(),
}),
};

Ok(AlterExpr {
Expand Down
31 changes: 30 additions & 1 deletion src/sql/src/parsers/alter_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::HashMap;

use common_query::AddColumnLocation;
use snafu::ResultExt;
use sqlparser::ast::{Expr, SqlOption};
use sqlparser::keywords::Keyword;
use sqlparser::parser::ParserError;
use sqlparser::tokenizer::Token;
Expand Down Expand Up @@ -94,9 +97,35 @@ impl<'a> ParserContext<'a> {
}
};
AlterTableOperation::RenameTable { new_table_name }
} else if self.parser.parse_keyword(Keyword::SET) {
let _ = self.parser.parse_keyword(Keyword::COLUMN);
let column_name = Self::canonicalize_identifier(self.parser.parse_identifier(false)?);

let _ = self.parser.parse_keyword(Keyword::FULLTEXT);

let options= self
.parser
.parse_options(Keyword::WITH)?
.into_iter()
.map(|option: SqlOption| -> Result<(String, String)> {
let (key, value) = (option.name, option.value);
let v = match value {
Expr::Value(sqlparser::ast::Value::SingleQuotedString(v))
| Expr::Value(sqlparser::ast::Value::DoubleQuotedString(v)) => v,
Expr::Identifier(v) => v.value,
Expr::Value(sqlparser::ast::Value::Number(v, _)) => v.to_string(),
_ => return error::InvalidTableOptionValueSnafu { key, value }.fail(),
};
let k = key.value.to_lowercase();
Ok((k, v))
})
.collect::<Result<HashMap<String, String>>>()
.unwrap();

AlterTableOperation::AlterColumnFulltext { column_name, options: options.into() }
} else {
return Err(ParserError::ParserError(format!(
"expect keyword ADD or DROP or MODIFY or RENAME after ALTER TABLE, found {}",
"expect keyword ADD or DROP or MODIFY or RENAME or SET after ALTER TABLE, found {}",
self.parser.peek_token()
)));
};
Expand Down
16 changes: 15 additions & 1 deletion src/sql/src/statements/alter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::fmt::{Debug, Display};
use std::{collections::HashMap, fmt::{Debug, Display}, ops::ControlFlow};

use common_query::AddColumnLocation;
use sqlparser::ast::{ColumnDef, DataType, Ident, ObjectName, TableConstraint};
use sqlparser_derive::{Visit, VisitMut};
use sqlparser::ast::{Visit, VisitMut, Visitor, VisitorMut};

use super::OptionMap;

#[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut)]
pub struct AlterTable {
Expand Down Expand Up @@ -71,6 +74,11 @@ pub enum AlterTableOperation {
DropColumn { name: Ident },
/// `RENAME <new_table_name>`
RenameTable { new_table_name: String },
/// `SET COLUMN <column_name> FULLTEXT WITH`
AlterColumnFulltext {
column_name: Ident,
options: OptionMap,
},
}

impl Display for AlterTableOperation {
Expand All @@ -97,6 +105,12 @@ impl Display for AlterTableOperation {
} => {
write!(f, r#"MODIFY COLUMN {column_name} {target_type}"#)
}
AlterTableOperation::AlterColumnFulltext {
column_name,
options,
} => {
write!(f, r#"SET COLUMN {column_name} FULLTEXT WITH {:?}"#, options.hash_options)
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/sql/src/statements/option_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const REDACTED_OPTIONS: [&str; 2] = ["access_key_id", "secret_access_key"];
pub struct OptionMap {
options: BTreeMap<String, String>,
secrets: BTreeMap<String, SecretString>,
pub hash_options: HashMap<String, String>, // TODO(renjj): move into a new struct
}

impl OptionMap {
Expand Down
12 changes: 11 additions & 1 deletion src/table/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ pub enum Error {
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Failed to retrieve fulltext options from column metadata"))]
FulltextOptions {
#[snafu(implicit)]
location: Location,
source: datatypes::error::Error,
column_name: String,
},
}

impl ErrorExt for Error {
Expand All @@ -155,7 +163,9 @@ impl ErrorExt for Error {
Error::TableOperation { source } => source.status_code(),
Error::ColumnNotExists { .. } => StatusCode::TableColumnNotFound,
Error::Unsupported { .. } => StatusCode::Unsupported,
Error::ParseTableOption { .. } => StatusCode::InvalidArguments,
Error::ParseTableOption { .. } | Error::FulltextOptions { .. } => {
StatusCode::InvalidArguments
}
Error::MissingTimeIndexColumn { .. } => StatusCode::IllegalState,
}
}
Expand Down
41 changes: 40 additions & 1 deletion src/table/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME};
use common_query::AddColumnLocation;
use datafusion_expr::TableProviderFilterPushDown;
pub use datatypes::error::{Error as ConvertError, Result as ConvertResult};
use datatypes::schema::{ColumnSchema, RawSchema, Schema, SchemaBuilder, SchemaRef};
use datatypes::schema::{
ColumnSchema, FulltextOptions, RawSchema, Schema, SchemaBuilder, SchemaRef,
};
use derive_builder::Builder;
use serde::{Deserialize, Serialize};
use snafu::{ensure, OptionExt, ResultExt};
Expand Down Expand Up @@ -208,6 +210,10 @@ impl TableMeta {
.next_column_id(self.next_column_id);
Ok(meta_builder)
}
AlterKind::AddFulltexts {
column_name,
options,
} => self.change_column_fulltext(table_name, column_name, options.clone()),
}
}

Expand Down Expand Up @@ -588,6 +594,39 @@ impl TableMeta {
Ok(meta_builder)
}

fn change_column_fulltext(
&self,
table_name: &str,
column_name: &String,
options: HashMap<String, String>,
) -> Result<TableMetaBuilder> {
let table_schema = &self.schema;
let mut meta_builder = self.new_meta_builder();

let index = table_schema
.column_index_by_name(column_name)
.with_context(|| error::ColumnNotExistsSnafu {
column_name,
table_name,
})?;
let mut column = &table_schema.column_schemas()[index];

let fulltext_options = column
.fulltext_options()
.context(error::FulltextOptionsSnafu { column_name })?;

let mut fulltext_options = match fulltext_options {
Some(fulltext_options) => fulltext_options,
None => FulltextOptions::default(),
};

let new_column = column.clone()
.with_fulltext_options(fulltext_options)
.context(error::FulltextOptionsSnafu { column_name })?;

Ok(meta_builder)
}

/// Split requests into different groups using column location info.
fn split_requests_by_column_location<'a>(
&self,
Expand Down
4 changes: 4 additions & 0 deletions src/table/src/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ pub enum AlterKind {
RenameTable {
new_table_name: String,
},
AddFulltexts {
column_name: String,
options: HashMap<String, String>,
}
}

#[derive(Debug)]
Expand Down

0 comments on commit 693130c

Please sign in to comment.