Skip to content

feat: allow multiple actions in one ALTER TABLE statement #960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2023
Merged

feat: allow multiple actions in one ALTER TABLE statement #960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,9 @@ pub enum Statement {
/// Table name
#[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]
name: ObjectName,
operation: AlterTableOperation,
if_exists: bool,
only: bool,
operations: Vec<AlterTableOperation>,
},
AlterIndex {
name: ObjectName,
Expand Down Expand Up @@ -2602,8 +2604,24 @@ impl fmt::Display for Statement {
}
Ok(())
}
Statement::AlterTable { name, operation } => {
write!(f, "ALTER TABLE {name} {operation}")
Statement::AlterTable {
name,
if_exists,
only,
operations,
} => {
write!(f, "ALTER TABLE ")?;
if *if_exists {
write!(f, "IF EXISTS ")?;
}
if *only {
write!(f, "ONLY ")?;
}
write!(
f,
"{name} {operations}",
operations = display_comma_separated(operations)
)
}
Statement::AlterIndex { name, operation } => {
write!(f, "ALTER INDEX {name} {operation}")
Expand Down
342 changes: 174 additions & 168 deletions src/parser/mod.rs
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions src/test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,26 @@ pub fn expr_from_projection(item: &SelectItem) -> &Expr {
}
}

pub fn alter_table_op_with_name(stmt: Statement, expected_name: &str) -> AlterTableOperation {
match stmt {
Statement::AlterTable {
name,
if_exists,
only: is_only,
operations,
} => {
assert_eq!(name.to_string(), expected_name);
assert!(!if_exists);
assert!(!is_only);
only(operations)
}
_ => panic!("Expected ALTER TABLE statement"),
}
}
pub fn alter_table_op(stmt: Statement) -> AlterTableOperation {
alter_table_op_with_name(stmt, "tab")
}

/// Creates a `Value::Number`, panic'ing if n is not a number
pub fn number(n: &str) -> Value {
Value::Number(n.parse().unwrap(), false)
Expand Down
179 changes: 67 additions & 112 deletions tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use sqlparser::dialect::{
use sqlparser::keywords::ALL_KEYWORDS;
use sqlparser::parser::{Parser, ParserError, ParserOptions};
use test_utils::{
all_dialects, assert_eq_vec, expr_from_projection, join, number, only, table, table_alias,
TestedDialects,
all_dialects, alter_table_op, assert_eq_vec, expr_from_projection, join, number, only, table,
table_alias, TestedDialects,
};

#[macro_use]
Expand Down Expand Up @@ -2915,48 +2915,37 @@ fn parse_create_external_table_lowercase() {
#[test]
fn parse_alter_table() {
let add_column = "ALTER TABLE tab ADD COLUMN foo TEXT;";
match one_statement_parses_to(add_column, "ALTER TABLE tab ADD COLUMN foo TEXT") {
Statement::AlterTable {
name,
operation:
AlterTableOperation::AddColumn {
column_keyword,
if_not_exists,
column_def,
},
match alter_table_op(one_statement_parses_to(
add_column,
"ALTER TABLE tab ADD COLUMN foo TEXT",
)) {
AlterTableOperation::AddColumn {
column_keyword,
if_not_exists,
column_def,
} => {
assert!(column_keyword);
assert!(!if_not_exists);
assert_eq!("tab", name.to_string());
assert_eq!("foo", column_def.name.to_string());
assert_eq!("TEXT", column_def.data_type.to_string());
}
_ => unreachable!(),
};

let rename_table = "ALTER TABLE tab RENAME TO new_tab";
match verified_stmt(rename_table) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice refactoring of the tests -- thank you

Statement::AlterTable {
name,
operation: AlterTableOperation::RenameTable { table_name },
} => {
assert_eq!("tab", name.to_string());
assert_eq!("new_tab", table_name.to_string())
match alter_table_op(verified_stmt(rename_table)) {
AlterTableOperation::RenameTable { table_name } => {
assert_eq!("new_tab", table_name.to_string());
}
_ => unreachable!(),
};

let rename_column = "ALTER TABLE tab RENAME COLUMN foo TO new_foo";
match verified_stmt(rename_column) {
Statement::AlterTable {
name,
operation:
AlterTableOperation::RenameColumn {
old_column_name,
new_column_name,
},
match alter_table_op(verified_stmt(rename_column)) {
AlterTableOperation::RenameColumn {
old_column_name,
new_column_name,
} => {
assert_eq!("tab", name.to_string());
assert_eq!(old_column_name.to_string(), "foo");
assert_eq!(new_column_name.to_string(), "new_foo");
}
Expand Down Expand Up @@ -3042,21 +3031,15 @@ fn parse_alter_view_with_columns() {

#[test]
fn parse_alter_table_add_column() {
match verified_stmt("ALTER TABLE tab ADD foo TEXT") {
Statement::AlterTable {
operation: AlterTableOperation::AddColumn { column_keyword, .. },
..
} => {
match alter_table_op(verified_stmt("ALTER TABLE tab ADD foo TEXT")) {
AlterTableOperation::AddColumn { column_keyword, .. } => {
assert!(!column_keyword);
}
_ => unreachable!(),
};

match verified_stmt("ALTER TABLE tab ADD COLUMN foo TEXT") {
Statement::AlterTable {
operation: AlterTableOperation::AddColumn { column_keyword, .. },
..
} => {
match alter_table_op(verified_stmt("ALTER TABLE tab ADD COLUMN foo TEXT")) {
AlterTableOperation::AddColumn { column_keyword, .. } => {
assert!(column_keyword);
}
_ => unreachable!(),
Expand All @@ -3075,24 +3058,19 @@ fn parse_alter_table_add_column_if_not_exists() {
options: None,
};

match dialects.verified_stmt("ALTER TABLE tab ADD IF NOT EXISTS foo TEXT") {
Statement::AlterTable {
operation: AlterTableOperation::AddColumn { if_not_exists, .. },
..
} => {
match alter_table_op(dialects.verified_stmt("ALTER TABLE tab ADD IF NOT EXISTS foo TEXT")) {
AlterTableOperation::AddColumn { if_not_exists, .. } => {
assert!(if_not_exists);
}
_ => unreachable!(),
};

match dialects.verified_stmt("ALTER TABLE tab ADD COLUMN IF NOT EXISTS foo TEXT") {
Statement::AlterTable {
operation:
AlterTableOperation::AddColumn {
column_keyword,
if_not_exists,
..
},
match alter_table_op(
dialects.verified_stmt("ALTER TABLE tab ADD COLUMN IF NOT EXISTS foo TEXT"),
) {
AlterTableOperation::AddColumn {
column_keyword,
if_not_exists,
..
} => {
assert!(column_keyword);
Expand All @@ -3118,12 +3096,10 @@ fn parse_alter_table_constraints() {
check_one("CHECK (end_date > start_date OR end_date IS NULL)");

fn check_one(constraint_text: &str) {
match verified_stmt(&format!("ALTER TABLE tab ADD {constraint_text}")) {
Statement::AlterTable {
name,
operation: AlterTableOperation::AddConstraint(constraint),
} => {
assert_eq!("tab", name.to_string());
match alter_table_op(verified_stmt(&format!(
"ALTER TABLE tab ADD {constraint_text}"
))) {
AlterTableOperation::AddConstraint(constraint) => {
assert_eq!(constraint_text, constraint.to_string());
}
_ => unreachable!(),
Expand All @@ -3145,17 +3121,12 @@ fn parse_alter_table_drop_column() {
);

fn check_one(constraint_text: &str) {
match verified_stmt(&format!("ALTER TABLE tab {constraint_text}")) {
Statement::AlterTable {
name,
operation:
AlterTableOperation::DropColumn {
column_name,
if_exists,
cascade,
},
match alter_table_op(verified_stmt(&format!("ALTER TABLE tab {constraint_text}"))) {
AlterTableOperation::DropColumn {
column_name,
if_exists,
cascade,
} => {
assert_eq!("tab", name.to_string());
assert_eq!("is_active", column_name.to_string());
assert!(if_exists);
assert!(cascade);
Expand All @@ -3168,12 +3139,10 @@ fn parse_alter_table_drop_column() {
#[test]
fn parse_alter_table_alter_column() {
let alter_stmt = "ALTER TABLE tab";
match verified_stmt(&format!("{alter_stmt} ALTER COLUMN is_active SET NOT NULL")) {
Statement::AlterTable {
name,
operation: AlterTableOperation::AlterColumn { column_name, op },
} => {
assert_eq!("tab", name.to_string());
match alter_table_op(verified_stmt(&format!(
"{alter_stmt} ALTER COLUMN is_active SET NOT NULL"
))) {
AlterTableOperation::AlterColumn { column_name, op } => {
assert_eq!("is_active", column_name.to_string());
assert_eq!(op, AlterColumnOperation::SetNotNull {});
}
Expand All @@ -3185,14 +3154,10 @@ fn parse_alter_table_alter_column() {
"ALTER TABLE tab ALTER COLUMN is_active DROP NOT NULL",
);

match verified_stmt(&format!(
match alter_table_op(verified_stmt(&format!(
"{alter_stmt} ALTER COLUMN is_active SET DEFAULT false"
)) {
Statement::AlterTable {
name,
operation: AlterTableOperation::AlterColumn { column_name, op },
} => {
assert_eq!("tab", name.to_string());
))) {
AlterTableOperation::AlterColumn { column_name, op } => {
assert_eq!("is_active", column_name.to_string());
assert_eq!(
op,
Expand All @@ -3204,12 +3169,10 @@ fn parse_alter_table_alter_column() {
_ => unreachable!(),
}

match verified_stmt(&format!("{alter_stmt} ALTER COLUMN is_active DROP DEFAULT")) {
Statement::AlterTable {
name,
operation: AlterTableOperation::AlterColumn { column_name, op },
} => {
assert_eq!("tab", name.to_string());
match alter_table_op(verified_stmt(&format!(
"{alter_stmt} ALTER COLUMN is_active DROP DEFAULT"
))) {
AlterTableOperation::AlterColumn { column_name, op } => {
assert_eq!("is_active", column_name.to_string());
assert_eq!(op, AlterColumnOperation::DropDefault {});
}
Expand All @@ -3220,12 +3183,10 @@ fn parse_alter_table_alter_column() {
#[test]
fn parse_alter_table_alter_column_type() {
let alter_stmt = "ALTER TABLE tab";
match verified_stmt("ALTER TABLE tab ALTER COLUMN is_active SET DATA TYPE TEXT") {
Statement::AlterTable {
name,
operation: AlterTableOperation::AlterColumn { column_name, op },
} => {
assert_eq!("tab", name.to_string());
match alter_table_op(verified_stmt(
"ALTER TABLE tab ALTER COLUMN is_active SET DATA TYPE TEXT",
)) {
AlterTableOperation::AlterColumn { column_name, op } => {
assert_eq!("is_active", column_name.to_string());
assert_eq!(
op,
Expand Down Expand Up @@ -3260,34 +3221,28 @@ fn parse_alter_table_alter_column_type() {
#[test]
fn parse_alter_table_drop_constraint() {
let alter_stmt = "ALTER TABLE tab";
match verified_stmt("ALTER TABLE tab DROP CONSTRAINT constraint_name CASCADE") {
Statement::AlterTable {
name,
operation:
AlterTableOperation::DropConstraint {
name: constr_name,
if_exists,
cascade,
},
match alter_table_op(verified_stmt(
"ALTER TABLE tab DROP CONSTRAINT constraint_name CASCADE",
)) {
AlterTableOperation::DropConstraint {
name: constr_name,
if_exists,
cascade,
} => {
assert_eq!("tab", name.to_string());
assert_eq!("constraint_name", constr_name.to_string());
assert!(!if_exists);
assert!(cascade);
}
_ => unreachable!(),
}
match verified_stmt("ALTER TABLE tab DROP CONSTRAINT IF EXISTS constraint_name") {
Statement::AlterTable {
name,
operation:
AlterTableOperation::DropConstraint {
name: constr_name,
if_exists,
cascade,
},
match alter_table_op(verified_stmt(
"ALTER TABLE tab DROP CONSTRAINT IF EXISTS constraint_name",
)) {
AlterTableOperation::DropConstraint {
name: constr_name,
if_exists,
cascade,
} => {
assert_eq!("tab", name.to_string());
assert_eq!("constraint_name", constr_name.to_string());
assert!(if_exists);
assert!(!cascade);
Expand Down
Loading