From f73a330bd3b7b88b3f05e31bb06dfd627545d9e5 Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Sun, 29 Jun 2025 15:32:57 +0200 Subject: [PATCH] Add support for ALTER COLUMN TYPE in Redshift --- src/ast/ddl.rs | 20 +++++++++++++++----- src/ast/spans.rs | 1 + src/parser/mod.rs | 28 ++++++++++++++++++---------- tests/sqlparser_common.rs | 19 +------------------ tests/sqlparser_postgres.rs | 6 ++---- tests/sqlparser_redshift.rs | 5 +++++ 6 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index afe9f228..6f29d74e 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -893,7 +893,10 @@ pub enum AlterColumnOperation { data_type: DataType, /// PostgreSQL specific using: Option, + /// Whether the statement included the optional `SET DATA` keywords + had_set: bool, }, + /// `ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY [ ( sequence_options ) ]` /// /// Note: this is a PostgreSQL-specific operation. @@ -914,12 +917,19 @@ impl fmt::Display for AlterColumnOperation { AlterColumnOperation::DropDefault => { write!(f, "DROP DEFAULT") } - AlterColumnOperation::SetDataType { data_type, using } => { - if let Some(expr) = using { - write!(f, "SET DATA TYPE {data_type} USING {expr}") - } else { - write!(f, "SET DATA TYPE {data_type}") + AlterColumnOperation::SetDataType { + data_type, + using, + had_set, + } => { + if *had_set { + write!(f, "SET DATA ")?; } + write!(f, "TYPE {data_type}")?; + if let Some(expr) = using { + write!(f, " USING {expr}")?; + } + Ok(()) } AlterColumnOperation::AddGenerated { generated_as, diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 78ed772b..00882602 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -924,6 +924,7 @@ impl Spanned for AlterColumnOperation { AlterColumnOperation::SetDataType { data_type: _, using, + had_set: _, } => using.as_ref().map_or(Span::empty(), |u| u.span()), AlterColumnOperation::AddGenerated { .. } => Span::empty(), } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index a079488f..824cb861 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8734,16 +8734,10 @@ impl<'a> Parser<'a> { } } else if self.parse_keywords(&[Keyword::DROP, Keyword::DEFAULT]) { AlterColumnOperation::DropDefault {} - } else if self.parse_keywords(&[Keyword::SET, Keyword::DATA, Keyword::TYPE]) - || (is_postgresql && self.parse_keyword(Keyword::TYPE)) - { - let data_type = self.parse_data_type()?; - let using = if is_postgresql && self.parse_keyword(Keyword::USING) { - Some(self.parse_expr()?) - } else { - None - }; - AlterColumnOperation::SetDataType { data_type, using } + } else if self.parse_keywords(&[Keyword::SET, Keyword::DATA, Keyword::TYPE]) { + self.parse_set_data_type(true)? + } else if self.parse_keyword(Keyword::TYPE) { + self.parse_set_data_type(false)? } else if self.parse_keywords(&[Keyword::ADD, Keyword::GENERATED]) { let generated_as = if self.parse_keyword(Keyword::ALWAYS) { Some(GeneratedAs::Always) @@ -8909,6 +8903,20 @@ impl<'a> Parser<'a> { Ok(operation) } + fn parse_set_data_type(&mut self, had_set: bool) -> Result { + let data_type = self.parse_data_type()?; + let using = if self.parse_keyword(Keyword::USING) { + Some(self.parse_expr()?) + } else { + None + }; + Ok(AlterColumnOperation::SetDataType { + data_type, + using, + had_set, + }) + } + fn parse_part_or_partition(&mut self) -> Result { let keyword = self.expect_one_of_keywords(&[Keyword::PART, Keyword::PARTITION])?; match keyword { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index f6f51459..40e5ef5d 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -5046,7 +5046,6 @@ fn parse_alter_table_alter_column() { #[test] fn parse_alter_table_alter_column_type() { - let alter_stmt = "ALTER TABLE tab"; match alter_table_op(verified_stmt( "ALTER TABLE tab ALTER COLUMN is_active SET DATA TYPE TEXT", )) { @@ -5057,28 +5056,12 @@ fn parse_alter_table_alter_column_type() { AlterColumnOperation::SetDataType { data_type: DataType::Text, using: None, + had_set: true, } ); } _ => unreachable!(), } - - let dialect = TestedDialects::new(vec![Box::new(GenericDialect {})]); - - let res = - dialect.parse_sql_statements(&format!("{alter_stmt} ALTER COLUMN is_active TYPE TEXT")); - assert_eq!( - ParserError::ParserError("Expected: SET/DROP NOT NULL, SET DEFAULT, or SET DATA TYPE after ALTER COLUMN, found: TYPE".to_string()), - res.unwrap_err() - ); - - let res = dialect.parse_sql_statements(&format!( - "{alter_stmt} ALTER COLUMN is_active SET DATA TYPE TEXT USING 'text'" - )); - assert_eq!( - ParserError::ParserError("Expected: end of statement, found: USING".to_string()), - res.unwrap_err() - ); } #[test] diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 7b0a8c5d..e1a49c69 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -764,10 +764,7 @@ fn parse_drop_extension() { #[test] fn parse_alter_table_alter_column() { - pg().one_statement_parses_to( - "ALTER TABLE tab ALTER COLUMN is_active TYPE TEXT USING 'text'", - "ALTER TABLE tab ALTER COLUMN is_active SET DATA TYPE TEXT USING 'text'", - ); + pg().verified_stmt("ALTER TABLE tab ALTER COLUMN is_active TYPE TEXT USING 'text'"); match alter_table_op( pg().verified_stmt( @@ -783,6 +780,7 @@ fn parse_alter_table_alter_column() { AlterColumnOperation::SetDataType { data_type: DataType::Text, using: Some(using_expr), + had_set: true, } ); } diff --git a/tests/sqlparser_redshift.rs b/tests/sqlparser_redshift.rs index be2b6722..d7df678e 100644 --- a/tests/sqlparser_redshift.rs +++ b/tests/sqlparser_redshift.rs @@ -402,3 +402,8 @@ fn parse_extract_single_quotes() { fn parse_string_literal_backslash_escape() { redshift().one_statement_parses_to(r#"SELECT 'l\'auto'"#, "SELECT 'l''auto'"); } + +#[test] +fn test_alter_column_type() { + redshift().verified_stmt("ALTER TABLE customers ALTER COLUMN email TYPE TEXT"); +}