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

Autogenerate erroneously considers tables belonging to the default schema to be newly added. #170

Closed
sqlalchemy-bot opened this Issue Jan 30, 2014 · 7 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Jan 30, 2014

Migrated issue, originally created by Ben Beanfield (@benbeanfield)

In alembic.autogenerate.api._produce_net_changes(), the inspected default schema name is replaced with "None", however if some tables in metadata_table_names are explicitly associated with the default schema "foo" - such as with __table_args__ = { 'schema': 'foo' } - then these will be erroneously considered as new tables when compared with conn_table_names in _compare_tables() because "None" will not match "foo".

Relevant code snippet:

if include_schemas:
    schemas = set(inspector.get_schema_names())
    # replace default schema name with None
    schemas.discard("information_schema")
    # replace the "default" schema with None
    schemas.add(None)
    schemas.discard(connection.dialect.default_schema_name)

This will eventually result in this error:

File "$VIRTUALENV/alembic/autogenerate/compare.py", line 79, in _make_index
    unique=params['unique']
AttributeError: 'NoneType' object has no attribute 'c'

I've temporarily worked around this by changing including: connection.dialect.default_schema_name = 'public' in env.py. This works because my default schema is not 'public' and none of my relations are in the 'public' schema.


Environment:

Alembic 0.6.2

SQLAlchemy 0.9.1

Postgresql 9.2.6

Configuration: include_schemas=True

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 30, 2014

Michael Bayer (@zzzeek) wrote:

can you please be more specific as to what the search_path is set to, some samples of tables that are actually in the database and then some examples of exactly how the Table/MetaData is specific on the Python side. SQLAlchemy's "ignore the schema" use case should only apply to the case when we reflect a constraint, and the constraint points to another schema that is also in the search path. 0.9.2 will also provide a new flag ignore_schema_name=False to suit the use case where the user wants to have a complex search_path but still wants explicit schema names on Table objects.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 30, 2014

Michael Bayer (@zzzeek) wrote:

also note, just setting your search_path to "public", if even just for within env.py on the Connection used by autogenerate, should clear this up, assuming this is the usual issue of search_path names getting in the way.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 30, 2014

Ben Beanfield (@benbeanfield) wrote:

Hi Mike,
Thanks for the quick response!

Yep, changing the search_path to "public" does work, because _produce_net_changes() now replaces 'public' with None, and since 'public' is not used by any tables in the metadata, this substitution will not affect conn_table_names.

It appears to me that the code assumes that tables in the metadata that are associated with the default schema will not have an explicit schema configured via (for example) __table_args__.

This would explain why it replaces the default schema with None in conn_table_names. But, because it does not perform a similar replacement of schema names in metadata_table_names, this results in metadata_table_names.difference(conn_table_names) returning a set of all tables which have __table_args__ = {'schema': DEFAULT_SCHEMA_NAME}.

This appears to be intended to handle the case where users do not specify an explicit schema for their tables, but this fails where users specify an explicit schema for tables which are associated with their default schema.

Pseudo example with a existing database which contains tables "foo.car" and "bar.truck", and with a search path=foo:

class Car(Base):
   __table_args__ = { 'schema': 'foo' }
   # ...

class Truck(Base):
   __table_args__ = { 'schema': 'bar' }
   # ...

context.configure(
   # ...
   target_metadata=Base.metadata,
   include_schemas=True,
)

# in _produce_net_changes()
repr(conn_table_names)
set([(None, 'car'), ('bar', 'truck')])  
repr(metadata_table_names)
OrderedSet([('foo', 'car'), ('bar', 'truck')])


# in _compare_tables()
for s, tname in metadata_table_names.difference(conn_table_names):
   name = '%s.%s' % (s, tname) if s else tname
   print name
   'foo.car'
   # Treats "foo.car" as a newly added table.
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 30, 2014

Michael Bayer (@zzzeek) wrote:

It appears to me that the code assumes that tables in the metadata that are associated with the default schema will not have an explicit schema configured via (for example) table_args.

that is the case, yup. the schema arg on Table is specifically for those tables that require the "schema." prefix in order to be correctly identified. Note that "schema name" in most databases is a much simpler concept than that of Postgresql.

But, because it does not perform a similar replacement of schema names in metadata_table_names,

because again, the assumption is, you only use "schema" to refer to a Table that you specifically want to refer to as in an "other" schema. not the "base" schema.

but this fails where users specify an explicit schema for tables which are associated with their default schema.

pretty much.

why do you have "schema" set on Table objects that are known to be accessible via the single schema in your search_path? (or alternatively, why is that your search_path? )

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 31, 2014

Michael Bayer (@zzzeek) wrote:

can you please try out 4bd2739 and reopen if that doesn't fix the problem; I've made it so it makes the same "schema" adjustment when comparing the metadata tables as well. I've added tests which should be reproducing your condition and they now pass.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 31, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 31, 2014

Ben Beanfield (@benbeanfield) wrote:

That did it. Thank you.

@sqlalchemy-bot sqlalchemy-bot added the bug label Nov 27, 2018

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