Skip to content

Commit

Permalink
feat: alter column fulltext option
Browse files Browse the repository at this point in the history
Co-Authored-By: irenjj <renj.jiang@gmail.com>
  • Loading branch information
CookiePieWw and irenjj committed Nov 6, 2024
1 parent 809abdb commit 7cc1882
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 13 deletions.
8 changes: 8 additions & 0 deletions src/datatypes/src/schema/column_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ impl ColumnSchema {
);
Ok(self)
}

pub fn set_fulltext_options(&mut self, options: &FulltextOptions) -> Result<()> {
self.metadata.insert(
FULLTEXT_KEY.to_string(),
serde_json::to_string(options).context(error::SerializeSnafu)?,
);
Ok(())
}
}

impl TryFrom<&Field> for ColumnSchema {
Expand Down
70 changes: 63 additions & 7 deletions src/store-api/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ impl RegionMetadataBuilder {
AlterKind::ChangeColumnFulltext {
column_name,
options,
} => self.change_column_fulltext_options(column_name, options),
} => self.change_column_fulltext_options(column_name, options)?,
AlterKind::ChangeRegionOptions { options: _ } => {
// nothing to be done with RegionMetadata
}
Expand Down Expand Up @@ -630,11 +630,56 @@ impl RegionMetadataBuilder {
}
}

fn change_column_fulltext_options(&mut self, column_name: String, _options: FulltextOptions) {
fn change_column_fulltext_options(
&mut self,
column_name: String,
options: FulltextOptions,
) -> Result<()> {
for column_meta in self.column_metadatas.iter_mut() {
if column_meta.column_schema.name == column_name {}
if column_meta.column_schema.name == column_name {
ensure!(
column_meta.column_schema.data_type.is_string(),
SetFulltextOptionsSnafu {
column_name: column_name.to_string(),
reason: format!(
"column {} is not a string type, but {:?}",
column_name, column_meta.column_schema.data_type
),
}
);

let current_fulltext_options =
column_meta.column_schema.fulltext_options().map_err(|e| {
SetFulltextOptionsSnafu {
column_name: column_name.clone(),
reason: e.to_string(),
}
.build()
})?;

// Don't allow to change fulltext options if it is already enabled.
if !(current_fulltext_options.is_some_and(|o| o.enable) && options.enable) {
column_meta
.column_schema
.set_fulltext_options(&options)
.map_err(|e| {
SetFulltextOptionsSnafu {
column_name: column_name.clone(),
reason: e.to_string(),
}
.build()
})?;
} else {
return SetFulltextOptionsSnafu {
column_name,
reason: "fulltext options already enabled".to_string(),
}
.fail();
}
break;
}
}
todo!()
Ok(())
}
}

Expand Down Expand Up @@ -761,9 +806,20 @@ pub enum MetadataError {
location: Location,
},

#[snafu(display("Fail to parse fulltext analyzer proto"))]
InvalidFulltextAnalyzerProto {
source: api::error::Error,
#[snafu(display("Invalid change fulltext option request"))]
InvalidChangeFulltextOptionRequest {
#[snafu(source)]
error: api::error::Error,
},

#[snafu(display(
"Failed to set fulltext options for column {}, reason: {}",
column_name,
reason
))]
SetFulltextOptions {
column_name: String,
reason: String,
#[snafu(implicit)]
location: Location,
},
Expand Down
4 changes: 2 additions & 2 deletions src/store-api/src/region_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use strum::IntoStaticStr;

use crate::logstore::entry;
use crate::metadata::{
ColumnMetadata, InvalidFulltextAnalyzerProtoSnafu, InvalidRawRegionRequestSnafu,
ColumnMetadata, InvalidChangeFulltextOptionRequestSnafu, InvalidRawRegionRequestSnafu,
InvalidRegionOptionChangeRequestSnafu, InvalidRegionRequestSnafu, MetadataError,
RegionMetadata, Result,
};
Expand Down Expand Up @@ -538,7 +538,7 @@ impl TryFrom<alter_request::Kind> for AlterKind {
options: FulltextOptions {
enable: x.enable,
analyzer: try_as_fulltext_option(x.analyzer)
.context(InvalidFulltextAnalyzerProtoSnafu)?,
.context(InvalidChangeFulltextOptionRequestSnafu)?,
case_sensitive: x.case_sensitive,
},
},
Expand Down
13 changes: 13 additions & 0 deletions src/table/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@ pub enum Error {

#[snafu(display("Table options value is not valid, key: `{}`, value: `{}`", key, value))]
InvalidTableOptionValue { key: String, value: String },

#[snafu(display(
"Failed to set fulltext options for column {}, reason: {}",
column_name,
reason
))]
SetFulltextOptions {
column_name: String,
reason: String,
#[snafu(implicit)]
location: Location,
},
}

impl ErrorExt for Error {
Expand All @@ -156,6 +168,7 @@ impl ErrorExt for Error {
Error::ColumnExists { .. } => StatusCode::TableColumnExists,
Error::SchemaBuild { source, .. } => source.status_code(),
Error::TableOperation { source } => source.status_code(),
Error::SetFulltextOptions { .. } => StatusCode::InvalidArguments,
Error::ColumnNotExists { .. } => StatusCode::TableColumnNotFound,
Error::Unsupported { .. } => StatusCode::Unsupported,
Error::ParseTableOption { .. } => StatusCode::InvalidArguments,
Expand Down
86 changes: 82 additions & 4 deletions src/table/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl TableMeta {
AlterKind::ChangeColumnFulltext {
column_name,
options,
} => self.change_column_fulltext_options(column_name, options),
} => self.change_column_fulltext_options(table_name, column_name, options),
}
}

Expand Down Expand Up @@ -248,10 +248,88 @@ impl TableMeta {
/// Creates a [TableMetaBuilder] with modified column fulltext options.
fn change_column_fulltext_options(
&self,
_column_name: &str,
_options: &FulltextOptions,
table_name: &str,
column_name: &str,
options: &FulltextOptions,
) -> Result<TableMetaBuilder> {
todo!()
let table_schema = &self.schema;
let mut meta_builder = self.new_meta_builder();

let column = &table_schema
.column_schema_by_name(column_name)
.with_context(|| error::ColumnNotExistsSnafu {
column_name,
table_name,
})?;

ensure!(
column.data_type.is_string(),
error::SetFulltextOptionsSnafu {
column_name: column_name.to_string(),
reason: format!(
"column {} is not a string type, but {:?}",
column_name, column.data_type
),
}
);

let current_fulltext_options = column.fulltext_options().map_err(|e| {
error::SetFulltextOptionsSnafu {
column_name: column_name.to_string(),
reason: e.to_string(),
}
.build()
})?;

ensure!(
!(current_fulltext_options.is_some_and(|o| o.enable) && options.enable),
error::SetFulltextOptionsSnafu {
column_name: column_name.to_string(),
reason: "fulltext options already enabled".to_string(),
}
);

let mut columns = vec![];
for column_schema in table_schema.column_schemas() {
if column_schema.name == column_name {
let mut new_column_schema = column_schema.clone();
new_column_schema
.set_fulltext_options(options)
.map_err(|e| {
error::SetFulltextOptionsSnafu {
column_name: column_name.to_string(),
reason: e.to_string(),
}
.build()
})?;
columns.push(new_column_schema);
} else {
columns.push(column_schema.clone());
}
}

// TODO(CookiePieWw): This part for all alter table operations is similar. We can refactor it.
let mut builder = SchemaBuilder::try_from_columns(columns)
.with_context(|_| error::SchemaBuildSnafu {
msg: format!("Failed to convert column schemas into schema for table {table_name}"),
})?
.version(table_schema.version() + 1);

for (k, v) in table_schema.metadata().iter() {
builder = builder.add_metadata(k, v);
}

let new_schema = builder.build().with_context(|_| error::SchemaBuildSnafu {
msg: format!(
"Table {table_name} cannot change fulltext options for column {column_name}",
),
})?;

let _ = meta_builder
.schema(Arc::new(new_schema))
.primary_key_indices(self.primary_key_indices.clone());

Ok(meta_builder)
}

/// Allocate a new column for the table.
Expand Down

0 comments on commit 7cc1882

Please sign in to comment.