Skip to content

Commit

Permalink
refactor: use UNSET instead of enable (#4983)
Browse files Browse the repository at this point in the history 
* refactor: use UNSERT instead of enable

* fix: test

* chore: comment
  • Loading branch information
@CookiePieWw
CookiePieWw authored Nov 13, 2024
1 parent 175fddb commit aa6c2de
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 48 deletions.
141 changes: 104 additions & 37 deletions src/sql/src/parsers/alter_parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
use std::collections::HashMap;

use common_query::AddColumnLocation;
use datatypes::schema::COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE;
use datatypes::schema::{FulltextOptions, COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE};
use snafu::{ensure, ResultExt};
use sqlparser::ast::Ident;
use sqlparser::keywords::Keyword;
use sqlparser::parser::{Parser, ParserError};
use sqlparser::tokenizer::Token;
Expand All @@ -28,10 +29,6 @@ use crate::statements::alter::{AlterTable, AlterTableOperation, ChangeTableOptio
use crate::statements::statement::Statement;
use crate::util::parse_option_string;

fn validate_column_fulltext_alter_option(key: &str) -> bool {
key == COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE || validate_column_fulltext_create_option(key)
}

impl ParserContext<'_> {
pub(crate) fn parse_alter(&mut self) -> Result<Statement> {
let alter_table = self.parse_alter_table()?;
Expand Down Expand Up @@ -153,40 +150,73 @@ impl ParserContext<'_> {
.context(error::SyntaxSnafu)?,
);

if self.parser.parse_keyword(Keyword::SET) {
self.parser
.expect_keyword(Keyword::FULLTEXT)
.context(error::SyntaxSnafu)?;

let options = self
.parser
.parse_options(Keyword::WITH)
.context(error::SyntaxSnafu)?
.into_iter()
.map(parse_option_string)
.collect::<Result<HashMap<String, String>>>()?;

for key in options.keys() {
ensure!(
validate_column_fulltext_alter_option(key),
InvalidColumnOptionSnafu {
name: column_name.to_string(),
msg: format!("invalid FULLTEXT option: {key}"),
}
);
match self.parser.peek_token().token {
Token::Word(w) => {
if w.value.eq_ignore_ascii_case("UNSET") {
let _ = self.parser.next_token();

self.parser
.expect_keyword(Keyword::FULLTEXT)
.context(error::SyntaxSnafu)?;

Ok(AlterTableOperation::ChangeColumnFulltext {
column_name,
options: FulltextOptions {
enable: false,
..Default::default()
},
})
} else if w.keyword == Keyword::SET {
self.parse_alter_column_fulltext(column_name)
} else {
let data_type = self.parser.parse_data_type().context(error::SyntaxSnafu)?;
Ok(AlterTableOperation::ChangeColumnType {
column_name,
target_type: data_type,
})
}
}
_ => self.expected(
"SET or UNSET or data type after MODIFY COLUMN",
self.parser.peek_token(),
)?,
}
}

Ok(AlterTableOperation::ChangeColumnFulltext {
column_name,
options: options.try_into().context(SetFulltextOptionSnafu)?,
})
} else {
let target_type = self.parser.parse_data_type().context(error::SyntaxSnafu)?;
Ok(AlterTableOperation::ChangeColumnType {
column_name,
target_type,
})
fn parse_alter_column_fulltext(&mut self, column_name: Ident) -> Result<AlterTableOperation> {
let _ = self.parser.next_token();

self.parser
.expect_keyword(Keyword::FULLTEXT)
.context(error::SyntaxSnafu)?;

let mut options = self
.parser
.parse_options(Keyword::WITH)
.context(error::SyntaxSnafu)?
.into_iter()
.map(parse_option_string)
.collect::<Result<HashMap<String, String>>>()?;

for key in options.keys() {
ensure!(
validate_column_fulltext_create_option(key),
InvalidColumnOptionSnafu {
name: column_name.to_string(),
msg: format!("invalid FULLTEXT option: {key}"),
}
);
}

options.insert(
COLUMN_FULLTEXT_CHANGE_OPT_KEY_ENABLE.to_string(),
"true".to_string(),
);

Ok(AlterTableOperation::ChangeColumnFulltext {
column_name,
options: options.try_into().context(SetFulltextOptionSnafu)?,
})
}
}

Expand Down Expand Up @@ -557,7 +587,7 @@ mod tests {

#[test]
fn test_parse_alter_column_fulltext() {
let sql = "ALTER TABLE test_table MODIFY COLUMN a SET FULLTEXT WITH(enable='true',analyzer='English',case_sensitive='false')";
let sql = "ALTER TABLE test_table MODIFY COLUMN a SET FULLTEXT WITH(analyzer='English',case_sensitive='false')";
let mut result =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
Expand Down Expand Up @@ -595,6 +625,43 @@ mod tests {
_ => unreachable!(),
}

let sql = "ALTER TABLE test_table MODIFY COLUMN a UNSET FULLTEXT";
let mut result =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
assert_eq!(1, result.len());
let statement = result.remove(0);
assert_matches!(statement, Statement::Alter { .. });
match statement {
Statement::Alter(alter_table) => {
assert_eq!("test_table", alter_table.table_name().0[0].value);

let alter_operation = alter_table.alter_operation();
assert_matches!(
alter_operation,
AlterTableOperation::ChangeColumnFulltext { .. }
);
match alter_operation {
AlterTableOperation::ChangeColumnFulltext {
column_name,
options,
} => {
assert_eq!("a", column_name.value);
assert_eq!(
FulltextOptions {
enable: false,
analyzer: FulltextAnalyzer::English,
case_sensitive: false
},
*options
);
}
_ => unreachable!(),
}
}
_ => unreachable!(),
}

let invalid_sql = "ALTER TABLE test_table MODIFY COLUMN a SET FULLTEXT WITH('abcd'='true')";
let result = ParserContext::create_with_dialect(
invalid_sql,
Expand Down
20 changes: 13 additions & 7 deletions src/sql/src/statements/alter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub enum AlterTableOperation {
DropColumn { name: Ident },
/// `RENAME <new_table_name>`
RenameTable { new_table_name: String },
/// `MODIFY COLUMN <column_name> SET FULLTEXT [WITH <options>]`
/// `MODIFY COLUMN <column_name> [SET | UNSET] FULLTEXT [WITH <options>]`
ChangeColumnFulltext {
column_name: Ident,
options: FulltextOptions,
Expand Down Expand Up @@ -126,10 +126,16 @@ impl Display for AlterTableOperation {
AlterTableOperation::ChangeColumnFulltext {
column_name,
options,
} => write!(
f,
r#"MODIFY COLUMN {column_name} SET FULLTEXT WITH({options})"#,
),
} => match options.enable {
true => {
write!(f, "MODIFY COLUMN {column_name} SET FULLTEXT WITH(analyzer={0}, case_sensitive={1})", options.analyzer, options.case_sensitive)?;
Ok(())
}
false => {
write!(f, "MODIFY COLUMN {column_name} UNSET FULLTEXT")?;
Ok(())
}
},
}
}
}
Expand Down Expand Up @@ -243,7 +249,7 @@ ALTER TABLE monitor RENAME monitor_new"#,
}
}

let sql = "ALTER TABLE monitor MODIFY COLUMN a SET FULLTEXT WITH(enable='true',analyzer='English',case_sensitive='false')";
let sql = "ALTER TABLE monitor MODIFY COLUMN a SET FULLTEXT WITH(analyzer='English',case_sensitive='false')";
let stmts =
ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
.unwrap();
Expand All @@ -255,7 +261,7 @@ ALTER TABLE monitor RENAME monitor_new"#,
let new_sql = format!("\n{}", set);
assert_eq!(
r#"
ALTER TABLE monitor MODIFY COLUMN a SET FULLTEXT WITH(enable=true, analyzer=English, case_sensitive=false)"#,
ALTER TABLE monitor MODIFY COLUMN a SET FULLTEXT WITH(analyzer=English, case_sensitive=false)"#,
&new_sql
);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/cases/standalone/common/alter/change_col_fulltext_options.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ SHOW CREATE TABLE test;
| | ) |
+-------+---------------------------------------------------------------------------------------+

ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(enable = 'false');
ALTER TABLE test MODIFY COLUMN message UNSET FULLTEXT;

Affected Rows: 0

Expand Down Expand Up @@ -140,7 +140,7 @@ ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(analyzer = 'Chinese', c

Error: 1004(InvalidArguments), Invalid column option, column name: message, error: FULLTEXT index options already enabled

ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(enable = 'false');
ALTER TABLE test MODIFY COLUMN message UNSET FULLTEXT;

Affected Rows: 0

Expand Down
4 changes: 2 additions & 2 deletions tests/cases/standalone/common/alter/change_col_fulltext_options.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ SELECT * FROM test WHERE MATCHES(message, 'hello');
-- SQLNESS ARG restart=true
SHOW CREATE TABLE test;

ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(enable = 'false');
ALTER TABLE test MODIFY COLUMN message UNSET FULLTEXT;

SHOW CREATE TABLE test;

Expand All @@ -39,7 +39,7 @@ SHOW CREATE TABLE test;

ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(analyzer = 'Chinese', case_sensitive = 'false');

ALTER TABLE test MODIFY COLUMN message SET FULLTEXT WITH(enable = 'false');
ALTER TABLE test MODIFY COLUMN message UNSET FULLTEXT;

SHOW CREATE TABLE test;

Expand Down

0 comments on commit aa6c2de

Please sign in to comment.