Skip to content
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.

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

batch alter operations won't add server_default #338

Closed
sqlalchemy-bot opened this issue Nov 21, 2015 · 9 comments
Closed

batch alter operations won't add server_default #338

sqlalchemy-bot opened this issue Nov 21, 2015 · 9 comments

Comments

@sqlalchemy-bot
Copy link

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Migrated issue, originally created by Anthony Ryan (@anthonyryan1)

server_default doesn't seem to work on batch alter operations. Tested on sqlite3.

Minimal test case

Prepare test table

def upgrade():
    op.create_table(
      'test',
      sa.Column('integer', sa.Integer, nullable=False, primary_key=True),
      sa.Column('string', sa.Text, nullable=False),
    )

Set a column default value

def upgrade():
    with op.batch_alter_table('test') as batch_op:
        batch_op.alter_column('string', server_default='default')

Expected result

CREATE TABLE test (
        integer INTEGER NOT NULL, 
        string TEXT DEFAULT 'default' NOT NULL, 
        PRIMARY KEY (integer)
);

Actual result

CREATE TABLE test (
        integer INTEGER NOT NULL, 
        string TEXT NOT NULL, 
        PRIMARY KEY (integer)
);
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Changes by Anthony Ryan (@anthonyryan1):

  • edited description
2 similar comments
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Changes by Anthony Ryan (@anthonyryan1):

  • edited description
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Changes by Anthony Ryan (@anthonyryan1):

  • edited description
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Michael Bayer (@zzzeek) wrote:

potential fix, if you care to test:

diff --git a/alembic/operations/batch.py b/alembic/operations/batch.py
index 769cd92..369d08d 100644
--- a/alembic/operations/batch.py
+++ b/alembic/operations/batch.py
@@ -265,7 +265,7 @@ class ApplyBatchImpl(object):
         if nullable is not None:
             existing.nullable = nullable
         if server_default is not False:
-            existing.server_default = server_default
+            sql_schema.DefaultClause(server_default)._set_parent(existing)
         if autoincrement is not None:
             existing.autoincrement = bool(autoincrement)
 

@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 21, 2015

Changes by Michael Bayer (@zzzeek):

  • set milestone to "fasttrack"
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 22, 2015

Anthony Ryan (@anthonyryan1) wrote:

Hello Mike,

Thank you for the very fast response. I can confirm that patch seems to resolve my issue.

@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 22, 2015

Michael Bayer (@zzzeek) wrote:

  • Fixed bug where the server_default parameter of alter_column()
    would not function correctly in batch mode.
    fixes #338

3105ed6

@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 22, 2015

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot
Copy link
Author

@sqlalchemy-bot sqlalchemy-bot commented Nov 22, 2015

Michael Bayer (@zzzeek) wrote:

great thanks for checking that !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.