Code review comments

This commit is contained in:
Yoav Cohen 2025-07-01 14:35:44 +02:00
parent 16639bc108
commit d76f5ddaec
6 changed files with 8 additions and 35 deletions

View file

@ -893,7 +893,7 @@ pub enum AlterColumnOperation {
data_type: DataType,
/// PostgreSQL specific
using: Option<Expr>,
/// Whether the statement included the optional `SET DATA` keywords
/// Set to true if the statement includes the `SET DATA TYPE` keywords
had_set: bool,
},

View file

@ -1061,18 +1061,11 @@ pub trait Dialect: Debug + Any {
false
}
/// Returns true if the dialect supports `ALTER TABLE tbl ALTER COLUMN col TYPE <type>`
/// without specifying `SET DATA` before `TYPE`.
///
/// - [Redshift](https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html#r_ALTER_TABLE-synopsis)
/// - [PostgreSQL](https://www.postgresql.org/docs/current/sql-altertable.html)
fn supports_alter_column_type_without_set(&self) -> bool {
false
}
/// Returns true if the dialect supports `ALTER TABLE tbl ALTER COLUMN col SET DATA TYPE <type> USING <exp>`
///
/// - [PostgreSQL](https://www.postgresql.org/docs/current/sql-altertable.html)
/// Returns true if the dialect supports the `USING` clause in an `ALTER COLUMN` statement.
/// Example:
/// ```sql
/// ALTER TABLE tbl ALTER COLUMN col SET DATA TYPE <type> USING <exp>`
/// ```
fn supports_alter_column_type_using(&self) -> bool {
false
}

View file

@ -259,10 +259,6 @@ impl Dialect for PostgreSqlDialect {
true
}
fn supports_alter_column_type_without_set(&self) -> bool {
true
}
fn supports_alter_column_type_using(&self) -> bool {
true
}

View file

@ -129,8 +129,4 @@ impl Dialect for RedshiftSqlDialect {
fn supports_string_literal_backslash_escape(&self) -> bool {
true
}
fn supports_alter_column_type_without_set(&self) -> bool {
true
}
}

View file

@ -8736,9 +8736,7 @@ impl<'a> Parser<'a> {
AlterColumnOperation::DropDefault {}
} else if self.parse_keywords(&[Keyword::SET, Keyword::DATA, Keyword::TYPE]) {
self.parse_set_data_type(true)?
} else if self.dialect.supports_alter_column_type_without_set()
&& self.parse_keyword(Keyword::TYPE)
{
} 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) {

View file

@ -5063,17 +5063,7 @@ fn parse_alter_table_alter_column_type() {
}
_ => unreachable!(),
}
let dialects = all_dialects_where(|d| d.supports_alter_column_type_without_set());
dialects.verified_stmt(&format!("{alter_stmt} ALTER COLUMN is_active TYPE TEXT"));
let dialects = all_dialects_except(|d| d.supports_alter_column_type_without_set());
let res =
dialects.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()
);
verified_stmt(&format!("{alter_stmt} ALTER COLUMN is_active TYPE TEXT"));
let dialects = all_dialects_where(|d| d.supports_alter_column_type_using());
dialects.verified_stmt(&format!(