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

Table not set for removed columns passed to include_object #200

Closed
sqlalchemy-bot opened this Issue Apr 30, 2014 · 6 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Apr 30, 2014

Migrated issue, originally created by Janne Vanhala (@jpvanhal)

I would like some columns, which are not in the model, not to be removed from the database when I run autogenerate. I tried using include_object function like this:

def include_object(object, name, type_, reflected, compare_to):
    if (
        type_ == 'column' and 
        object.table.name == 'some_table' and 
        object.name == 'some_column'
    ):
        return False
    return True

context.configure(
    # ...
    include_object = include_object
)

However, this fails because object.table is None for removed columns. Thus there is no way to know what table removed columns belong to.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Michael Bayer (@zzzeek) wrote:

can you confirm what I'm seeing is that in this case the "reflected" object is None, right? that's not right, "reflected" should be the Column we got from the database and it should have the table. Can you try this patch please:

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index cfa110a..2c590f3 100644
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -133,8 +133,9 @@ def _compare_columns(schema, tname, object_filters, conn_table, metadata_table,
                     nullable=conn_table.c[cname].nullable,
                     server_default=conn_table.c[cname].server_default
                 )
+        conn_col = conn_table.c[cname]
         if _run_filters(rem_col, cname,
-                                "column", True, None, object_filters):
+                                "column", True, conn_col, object_filters):
             diffs.append(
                 ("remove_column", schema, tname, rem_col)
             )

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Changes by Michael Bayer (@zzzeek):

  • added labels: high priority
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Janne Vanhala (@jpvanhal) wrote:

In my case "reflected" is True and "compare_to" is None.

After your patch, "reflected" is still True and "compare_to" is the column from the database and it has the table.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Michael Bayer (@zzzeek) wrote:

  • Fixed bug where the include_object() filter would not receive
    the original :class:.Column object when evaluating a database-only
    column to be dropped; the object would not include the parent
    :class:.Table nor other aspects of the column that are important
    for generating the "downgrade" case where the column is recreated.
    fixes #200

bc6971a

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Apr 30, 2014

Michael Bayer (@zzzeek) wrote:

OK so fix is different, the original way you're trying to use it is correct. the "reflected" flag is True, meaning the object you're getting is supposed to be the actual reflected object. I think at some point that "rem_col" was there to recreate a Column object that might not have been present but now it's not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment