From 7426729882fec56fcd532ac8da41cf8e90b9bf32 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 25 Oct 2017 15:24:29 -0400 Subject: [PATCH] Resolve Postgresql implicit indexes via duplicates_constraint Fixed bug where autogenerate would produce a DROP statement for the index implicitly created by a Postgresql EXCLUDE constraint, rather than skipping it as is the case for indexes implicitly generated by unique constraints. Makes use of SQLAlchemy 1.0.x's improved "duplicates index" metadata and requires at least SQLAlchemy version 1.0.x to function correctly. Change-Id: I7362c8045f69553c6090dd3cb236569b0c9b1e67 Fixes: #461 --- alembic/autogenerate/compare.py | 6 ++++-- alembic/ddl/postgresql.py | 22 ++++++++++++++-------- alembic/testing/requirements.py | 7 +++++++ docs/build/unreleased/461.rst | 11 +++++++++++ tests/requirements.py | 19 +++++++++++++++++++ tests/test_autogen_indexes.py | 25 +++++++++++++++++++++++++ 6 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 docs/build/unreleased/461.rst diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index f393c40b..3abb32df 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -185,12 +185,14 @@ def _compare_tables(conn_table_names, metadata_table_names, def _make_index(params, conn_table): - # TODO: add .info such as 'duplicates_constraint' - return sa_schema.Index( + ix = sa_schema.Index( params['name'], *[conn_table.c[cname] for cname in params['column_names']], unique=params['unique'] ) + if 'duplicates_constraint' in params: + ix.info['duplicates_constraint'] = params['duplicates_constraint'] + return ix def _make_unique_constraint(params, conn_table): diff --git a/alembic/ddl/postgresql.py b/alembic/ddl/postgresql.py index 83471aa3..e42bc810 100644 --- a/alembic/ddl/postgresql.py +++ b/alembic/ddl/postgresql.py @@ -149,19 +149,25 @@ class PostgresqlImpl(DefaultImpl): conn_indexes, metadata_unique_constraints, metadata_indexes): + conn_uniques_by_name = dict( (c.name, c) for c in conn_unique_constraints) conn_indexes_by_name = dict( (c.name, c) for c in conn_indexes) - # TODO: if SQLA 1.0, make use of "duplicates_constraint" - # metadata - doubled_constraints = dict( - (name, (conn_uniques_by_name[name], conn_indexes_by_name[name])) - for name in set(conn_uniques_by_name).intersection( - conn_indexes_by_name) - ) - for name, (uq, ix) in doubled_constraints.items(): + if not util.sqla_100: + doubled_constraints = set( + conn_indexes_by_name[name] + for name in set(conn_uniques_by_name).intersection( + conn_indexes_by_name) + ) + else: + doubled_constraints = set( + index for index in + conn_indexes if index.info.get('duplicates_constraint') + ) + + for ix in doubled_constraints: conn_indexes.remove(ix) for idx in list(metadata_indexes): diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py index 7564b06e..32645ed8 100644 --- a/alembic/testing/requirements.py +++ b/alembic/testing/requirements.py @@ -158,6 +158,13 @@ class SuiteRequirements(Requirements): "SQLAlchemy 0.9.4 or greater required" ) + @property + def sqlalchemy_100(self): + return exclusions.skip_if( + lambda config: not util.sqla_100, + "SQLAlchemy 1.0.0 or greater required" + ) + @property def sqlalchemy_1014(self): return exclusions.skip_if( diff --git a/docs/build/unreleased/461.rst b/docs/build/unreleased/461.rst new file mode 100644 index 00000000..0435b59c --- /dev/null +++ b/docs/build/unreleased/461.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, postgresql, autogenerate + :tickets: 461 + + Fixed bug where autogenerate would produce a DROP statement for the index + implicitly created by a Postgresql EXCLUDE constraint, rather than skipping + it as is the case for indexes implicitly generated by unique constraints. + Makes use of SQLAlchemy 1.0.x's improved "duplicates index" metadata and + requires at least SQLAlchemy version 1.0.x to function correctly. + + diff --git a/tests/requirements.py b/tests/requirements.py index bdfa8a93..76629877 100644 --- a/tests/requirements.py +++ b/tests/requirements.py @@ -116,6 +116,25 @@ class DefaultRequirements(SuiteRequirements): return exclusions.only_if(check_uuid_ossp) + def _has_pg_extension(self, name): + def check(config): + if not exclusions.against(config, "postgresql"): + return False + count = config.db.scalar( + "SELECT count(*) FROM pg_extension " + "WHERE extname='%s'" % name) + return bool(count) + return exclusions.only_if(check, "needs %s extension" % name) + + @property + def hstore(self): + return self._has_pg_extension("hstore") + + @property + def btree_gist(self): + return self._has_pg_extension("btree_gist") + + @property def autoincrement_on_composite_pk(self): return exclusions.skip_if(["sqlite"], "not supported by database") diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py index 97d0aa1e..e426a678 100644 --- a/tests/test_autogen_indexes.py +++ b/tests/test_autogen_indexes.py @@ -609,6 +609,31 @@ class PGUniqueIndexTest(AutogenerateUniqueIndexTest): diffs = self._fixture(m1, m2, include_schemas=True) eq_(diffs, []) + @config.requirements.sqlalchemy_100 + @config.requirements.btree_gist + def test_exclude_const_unchanged(self): + from sqlalchemy.dialects.postgresql import TSRANGE, ExcludeConstraint + + m1 = MetaData() + m2 = MetaData() + + Table( + 'add_excl', m1, + Column('id', Integer, primary_key=True), + Column('period', TSRANGE), + ExcludeConstraint(('period', '&&'), name='quarters_period_excl') + ) + + Table( + 'add_excl', m2, + Column('id', Integer, primary_key=True), + Column('period', TSRANGE), + ExcludeConstraint(('period', '&&'), name='quarters_period_excl') + ) + + diffs = self._fixture(m1, m2) + eq_(diffs, []) + def test_same_tname_two_schemas(self): m1 = MetaData() m2 = MetaData() -- 2.47.2