Open sqlalchemy-bot opened 7 years ago
Michael Bayer (@zzzeek) wrote:
Reading that doc, this is only for NDB cluster. Secondly, the documentation you link states pretty clearly that the ONLINE keyword is deprecated, so this is out of the gate a legacy option.
keywords like ONLINE can be added using recipes that intercept alembic.AlterTable to add that keyword, however I assume you want the ONLINE keyword conditionally, which would mean extending operations like op.alter_column() to do so. There seem to be a lack of hooks to easily intercept new arguments for a case like this without having to create an entirely new operation, which is also possible if you are truly in a bind for this (though you can just emit textual ALTER as well), so if anything I'd rather add extensibility for existing operations.
Michael Bayer (@zzzeek) wrote:
proposal is to add "contextual_ops" to all the ddl.base.AlterTable etc constructs, have dialect impls propagate these arguments from the operations to the DDL construct. All operations need to support **kw
(many already do). Then you can write a @compiles(AlterTable, "mysql")
recipe that looks inside of element.contextual_opts
for mysql_online=True
and this is easy.
a small part of the patch, would need to add **kw
everywhere it needs to be as well as propagation in all of the impls:
diff --git a/alembic/ddl/base.py b/alembic/ddl/base.py
index f4a525f..e74d116 100644
--- a/alembic/ddl/base.py
+++ b/alembic/ddl/base.py
@@ -24,15 +24,18 @@ class AlterTable(DDLElement):
"""
- def __init__(self, table_name, schema=None):
+ def __init__(self, table_name, schema=None, contextual_opts=None):
self.table_name = table_name
self.schema = schema
+ self.contextual_opts = contextual_opts
class RenameTable(AlterTable):
- def __init__(self, old_table_name, new_table_name, schema=None):
- super(RenameTable, self).__init__(old_table_name, schema=schema)
+ def __init__(self, old_table_name, new_table_name, schema=None,
+ contextual_opts=None):
+ super(RenameTable, self).__init__(old_table_name, schema=schema,
+ contextual_opts=contextual_opts)
self.new_table_name = new_table_name
@@ -41,8 +44,9 @@ class AlterColumn(AlterTable):
def __init__(self, name, column_name, schema=None,
existing_type=None,
existing_nullable=None,
- existing_server_default=None):
- super(AlterColumn, self).__init__(name, schema=schema)
+ existing_server_default=None, contextual_opts=None):
+ super(AlterColumn, self).__init__(name, schema=schema,
+ contextual_opts=contextual_opts)
self.column_name = column_name
self.existing_type = sqltypes.to_instance(existing_type) \
if existing_type is not None else None
diff --git a/alembic/ddl/mysql.py b/alembic/ddl/mysql.py
index 3af4fa6..538121e 100644
--- a/alembic/ddl/mysql.py
+++ b/alembic/ddl/mysql.py
@@ -45,7 +45,8 @@ class MySQLImpl(DefaultImpl):
default=server_default if server_default is not False
else existing_server_default,
autoincrement=autoincrement if autoincrement is not None
- else existing_autoincrement
+ else existing_autoincrement,
+ contextual_opts=kw
)
)
elif nullable is not None or \
@@ -64,7 +65,8 @@ class MySQLImpl(DefaultImpl):
default=server_default if server_default is not False
else existing_server_default,
autoincrement=autoincrement if autoincrement is not None
- else existing_autoincrement
+ else existing_autoincrement,
+ contextual_opts=kw
)
)
elif server_default is not False:
@@ -72,6 +74,7 @@ class MySQLImpl(DefaultImpl):
MySQLAlterDefault(
table_name, column_name, server_default,
schema=schema,
+ contextual_opts=kw
)
)
@@ -204,8 +207,8 @@ class MySQLImpl(DefaultImpl):
class MySQLAlterDefault(AlterColumn):
- def __init__(self, name, column_name, default, schema=None):
- super(AlterColumn, self).__init__(name, schema=schema)
+ def __init__(self, name, column_name, default, schema=None, **kw):
+ super(AlterColumn, self).__init__(name, schema=schema, **kw)
self.column_name = column_name
self.default = default
@@ -217,8 +220,8 @@ class MySQLChangeColumn(AlterColumn):
type_=None,
nullable=None,
default=False,
- autoincrement=None):
- super(AlterColumn, self).__init__(name, schema=schema)
+ autoincrement=None, **kw):
+ super(AlterColumn, self).__init__(name, schema=schema, **kw)
self.column_name = column_name
self.nullable = nullable
self.newname = newname
spanosgeorge wrote:
Hey, thanks for the prompt response!
Indeed, I should have linked probably to MariaDB's doc: https://mariadb.com/kb/en/mariadb/alter-table/.
For MariaDB ALTER ONLINE TABLE
is just a synonym for ALTER TABLE ... LOCK=NONE
.
I will try to see if I can get away with any of the temporary solutions you suggested in your first response, until your patch lands to a proper alembic release.
Thanks again.
Changes by Michael Bayer (@zzzeek):
Changes by Michael Bayer (@zzzeek):
Changes by Michael Bayer (@zzzeek):
Changes by Michael Bayer (@zzzeek):
Migrated issue, originally created by spanosgeorge
Hello, if I'm reading correctly the source code, right now alembic doesn't support running
ALTER ONLINE TABLE
DDL statements. Correct?If indeed that's the case, are there plans to support it in the future?
Relevant documentation from MySQL: https://dev.mysql.com/doc/refman/5.6/en/alter-table-online-operations.html