Skip to content

Commit

Permalink
Change on_cluster type from Option<String> to Option<Ident>
Browse files Browse the repository at this point in the history
  • Loading branch information
git-hulk committed Jul 29, 2024
1 parent b01e9ec commit 516714c
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/ast/dml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub struct CreateTable {
pub on_commit: Option<OnCommit>,
/// ClickHouse "ON CLUSTER" clause:
/// <https://clickhouse.com/docs/en/sql-reference/distributed-ddl/>
pub on_cluster: Option<String>,
pub on_cluster: Option<Ident>,
/// ClickHouse "PRIMARY KEY " clause.
/// <https://clickhouse.com/docs/en/sql-reference/statements/create/table/>
pub primary_key: Option<Box<Expr>>,
Expand Down
4 changes: 2 additions & 2 deletions src/ast/helpers/stmt_create_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub struct CreateTableBuilder {
pub default_charset: Option<String>,
pub collation: Option<String>,
pub on_commit: Option<OnCommit>,
pub on_cluster: Option<String>,
pub on_cluster: Option<Ident>,
pub primary_key: Option<Box<Expr>>,
pub order_by: Option<OneOrManyWithParens<Expr>>,
pub partition_by: Option<Box<Expr>>,
Expand Down Expand Up @@ -261,7 +261,7 @@ impl CreateTableBuilder {
self
}

pub fn on_cluster(mut self, on_cluster: Option<String>) -> Self {
pub fn on_cluster(mut self, on_cluster: Option<Ident>) -> Self {
self.on_cluster = on_cluster;
self
}
Expand Down
2 changes: 1 addition & 1 deletion src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2165,7 +2165,7 @@ pub enum Statement {
/// ClickHouse dialect supports `ON CLUSTER` clause for ALTER TABLE
/// For example: `ALTER TABLE table_name ON CLUSTER cluster_name ADD COLUMN c UInt32`
/// [ClickHouse](https://clickhouse.com/docs/en/sql-reference/statements/alter/update)
on_cluster: Option<String>,
on_cluster: Option<Ident>,
},
/// ```sql
/// ALTER INDEX
Expand Down
9 changes: 2 additions & 7 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5372,14 +5372,9 @@ impl<'a> Parser<'a> {
}
}

fn parse_optional_on_cluster(&mut self) -> Result<Option<String>, ParserError> {
fn parse_optional_on_cluster(&mut self) -> Result<Option<Ident>, ParserError> {
if self.parse_keywords(&[Keyword::ON, Keyword::CLUSTER]) {
let next_token = self.next_token();
match next_token.token {
Token::SingleQuotedString(s) => Ok(Some(format!("'{}'", s))),
Token::Word(s) => Ok(Some(s.to_string())),
_ => self.expected("identifier or cluster literal", next_token)?,
}
Ok(Some(self.parse_identifier(false)?))
} else {
Ok(None)
}
Expand Down
25 changes: 19 additions & 6 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3506,7 +3506,7 @@ fn parse_create_table_on_cluster() {
let sql = "CREATE TABLE t ON CLUSTER '{cluster}' (a INT, b INT)";
match generic.verified_stmt(sql) {
Statement::CreateTable(CreateTable { on_cluster, .. }) => {
assert_eq!(on_cluster.unwrap(), "'{cluster}'".to_string());
assert_eq!(on_cluster.unwrap().to_string(), "'{cluster}'".to_string());
}
_ => unreachable!(),
}
Expand All @@ -3515,7 +3515,7 @@ fn parse_create_table_on_cluster() {
let sql = "CREATE TABLE t ON CLUSTER my_cluster (a INT, b INT)";
match generic.verified_stmt(sql) {
Statement::CreateTable(CreateTable { on_cluster, .. }) => {
assert_eq!(on_cluster.unwrap(), "my_cluster".to_string());
assert_eq!(on_cluster.unwrap().to_string(), "my_cluster".to_string());
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -3824,13 +3824,26 @@ fn parse_alter_table() {

#[test]
fn test_alter_table_with_on_cluster() {
let sql = "ALTER TABLE t ON CLUSTER 'cluster' ADD CONSTRAINT bar PRIMARY KEY (baz)";
match all_dialects().verified_stmt(sql) {
match all_dialects()
.verified_stmt("ALTER TABLE t ON CLUSTER 'cluster' ADD CONSTRAINT bar PRIMARY KEY (baz)")
{
Statement::AlterTable {
name, on_cluster, ..
} => {
std::assert_eq!(name.to_string(), "t");
std::assert_eq!(on_cluster, Some(Ident::with_quote('\'', "cluster")));
}
_ => unreachable!(),
}

match all_dialects()
.verified_stmt("ALTER TABLE t ON CLUSTER cluster_name ADD CONSTRAINT bar PRIMARY KEY (baz)")
{
Statement::AlterTable {
name, on_cluster, ..
} => {
std::assert_eq!(name.to_string(), "t");
std::assert_eq!(on_cluster, Some("'cluster'".to_string()));
std::assert_eq!(on_cluster, Some(Ident::new("cluster_name")));
}
_ => unreachable!(),
}
Expand All @@ -3839,7 +3852,7 @@ fn test_alter_table_with_on_cluster() {
.parse_sql_statements("ALTER TABLE t ON CLUSTER 123 ADD CONSTRAINT bar PRIMARY KEY (baz)");
std::assert_eq!(
res.unwrap_err(),
ParserError::ParserError("Expected: identifier or cluster literal, found: 123".to_string())
ParserError::ParserError("Expected: identifier, found: 123".to_string())
)
}

Expand Down

0 comments on commit 516714c

Please sign in to comment.