adding multiple columns in a single `alter table` statement doesn't work

[vidmartin]

When I try to parse the following valid SQL statement (it runs fine on my MySQL server) via sqlparser:

alter table sometable add column b integer, add column c integer;

Instead of successfully parsing it, I get the following error:

Expected end of statement, found: ,

It seems like sqlparser doesn't understand having multiple additions in a single alter table statement. Is it possible to make this work?

[AugustoFKL]

@vidmartin will work on that.

As this isn't the ANSI behavior (1), wasn't a priority. I know MySQL supports it (2), and PostgreSQL (3) as well, right? Do you need it only for MySQL, or other dialects as well? If just for MySQL, I can work on that right away. If this needs to be generic, I'll have to study a little before implementing it.

[1] : https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#add-column-definition
[2] : https://dev.mysql.com/doc/refman/8.0/en/alter-table.html
[3] : https://www.postgresql.org/docs/current/sql-altertable.html

[ForbesLindesay]

I'd like to have a go at implementing this. I'd like both Postgres and MySQL support.

Looking at the documentation for Postgres, there are some "actions" that can appear in a list with multiple actions as part of a single statement, but there are some types of alter table statement that can only have a single action: https://www.postgresql.org/docs/15/sql-altertable.html

I can see two ways to implement this in terms of our types:

Option 1

Update Statement::AlterTable to take a list of operations:

    AlterTable {
        /// Table name
        #[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]
        name: ObjectName,
-       operation: AlterTableOperation,
+       operations: Vec<AlterTableOperation>,
    },

Option 2

Split the AlterTableOperation into two:

pub enum AlterTableOperation {
   /// Postgres/MySQL/some others allow multiple actions in a single statement.
   /// For other dialects, this should only contain one entry
   MultipleActions(Vec<AlterTableOperationAction>),

    /// `RENAME [ COLUMN ] <old_column_name> TO <new_column_name>`
    RenameColumn {
        old_column_name: Ident,
        new_column_name: Ident,
    },

    /// `RENAME CONSTRAINT <old_constraint_name> TO <new_constraint_name>`
    ///
    /// Note: this is a PostgreSQL-specific operation.
    RenameConstraint { old_name: Ident, new_name: Ident },


    // ... other actions that cannot be grouped here...
}

pub enum AlterTableOperationAction {
    AddColumn {
        /// `[COLUMN]`.
        column_keyword: bool,
        /// `[IF NOT EXISTS]`
        if_not_exists: bool,
        /// <column_def>.
        column_def: ColumnDef,
    },
    /// `DROP [ COLUMN ] [ IF EXISTS ] <column_name> [ CASCADE ]`
    DropColumn {
        column_name: Ident,
        if_exists: bool,
        cascade: bool,
    },
    /// `ALTER [ COLUMN ]`
    AlterColumn {
        column_name: Ident,
        op: AlterColumnOperation,
    },
    // ... other actions that can be grouped in a single ALTER TABLE statement here...
}

Conclusion

My instinct is to go with Option 1. It's more flexible and I think it will be more consistent across different database systems where some support grouping certain things and others don't. I think it's more in common with the ethos of this package, which tries to be agnostic to the semantics of the queries it parses.