From: Mike Bayer Date: Sat, 20 Sep 2014 21:00:21 +0000 (-0400) Subject: - Added a routine by which the Postgresql Alembic dialect inspects X-Git-Tag: rel_0_7_0~70 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ff74edfc7e1dab3f6e19c546eb956fbd0bae30d8;p=thirdparty%2Fsqlalchemy%2Falembic.git - Added a routine by which the Postgresql Alembic dialect inspects the server default of INTEGER/BIGINT columns as they are reflected during autogenerate for the pattern ``nextval(...)`` containing a potential sequence name, then queries ``pg_catalog`` to see if this sequence is "owned" by the column being reflected; if so, it assumes this is a SERIAL or BIGSERIAL column and the server default is omitted from the column reflection as well as any kind of server_default comparison or rendering, along with an INFO message in the logs indicating this has taken place. This allows SERIAL/BIGSERIAL columns to keep the SEQUENCE from being unnecessarily present within the autogenerate operation. fixes #73 --- diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index 4b640fcb..1f9cf77e 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -1,4 +1,5 @@ from sqlalchemy import schema as sa_schema, types as sqltypes +from sqlalchemy import event import logging from .. import compat from sqlalchemy.util import OrderedSet @@ -63,7 +64,12 @@ def _compare_tables(conn_table_names, metadata_table_names, name = sa_schema._get_table_key(tname, s) exists = name in removal_metadata.tables t = sa_schema.Table(tname, removal_metadata, schema=s) + if not exists: + event.listen( + t, + "column_reflect", + autogen_context['context'].impl.autogen_column_reflect) inspector.reflecttable(t, None) if _run_filters(t, tname, "table", True, None, object_filters): diffs.append(("remove_table", t)) @@ -78,6 +84,10 @@ def _compare_tables(conn_table_names, metadata_table_names, exists = name in existing_metadata.tables t = sa_schema.Table(tname, existing_metadata, schema=s) if not exists: + event.listen( + t, + "column_reflect", + autogen_context['context'].impl.autogen_column_reflect) inspector.reflecttable(t, None) conn_column_info[(s, tname)] = t diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py index bcfe864f..9f75acc6 100644 --- a/alembic/ddl/impl.py +++ b/alembic/ddl/impl.py @@ -231,6 +231,15 @@ class DefaultImpl(with_metaclass(ImplMeta)): metadata_indexes): pass + def autogen_column_reflect(self, inspector, table, column_info): + """A hook that is attached to the 'column_reflect' event for when + a Table is reflected from the database during the autogenerate + process. + + Dialects can elect to modify the information gathered here. + + """ + def start_migrations(self): """A hook called when :meth:`.EnvironmentContext.run_migrations` is called. diff --git a/alembic/ddl/postgresql.py b/alembic/ddl/postgresql.py index 6602d238..6193cda7 100644 --- a/alembic/ddl/postgresql.py +++ b/alembic/ddl/postgresql.py @@ -3,6 +3,11 @@ import re from .. import compat from .base import compiles, alter_table, format_table_name, RenameTable from .impl import DefaultImpl +from sqlalchemy.dialects.postgresql import INTEGER, BIGINT +from sqlalchemy import text +import logging + +log = logging.getLogger(__name__) class PostgresqlImpl(DefaultImpl): @@ -13,7 +18,6 @@ class PostgresqlImpl(DefaultImpl): metadata_column, rendered_metadata_default, rendered_inspector_default): - # don't do defaults for SERIAL columns if metadata_column.primary_key and \ metadata_column is metadata_column.table._autoincrement_column: @@ -37,6 +41,36 @@ class PostgresqlImpl(DefaultImpl): ) ) + def autogen_column_reflect(self, inspector, table, column_info): + if column_info.get('default') and \ + isinstance(column_info['type'], (INTEGER, BIGINT)): + seq_match = re.match( + r"nextval\('(.+?)'::regclass\)", + column_info['default']) + if seq_match: + info = inspector.bind.execute(text( + "select c.relname, a.attname " + "from pg_class as c join pg_depend d on d.objid=c.oid and " + "d.classid='pg_class'::regclass and " + "d.refclassid='pg_class'::regclass " + "join pg_class t on t.oid=d.refobjid " + "join pg_attribute a on a.attrelid=t.oid and " + "a.attnum=d.refobjsubid " + "where c.relkind='S' and c.relname=:seqname" + ), seqname=seq_match.group(1)).first() + if info: + seqname, colname = info + if colname == column_info['name']: + log.info( + "Detected sequence named '%s' as " + "owned by integer column '%s(%s)', " + "assuming SERIAL and omitting" % ( + seqname, table.name, colname + )) + # sequence, and the owner is this column, + # its a SERIAL - whack it! + del column_info['default'] + @compiles(RenameTable, "postgresql") def visit_rename_table(element, compiler, **kw): diff --git a/alembic/testing/__init__.py b/alembic/testing/__init__.py index 7bdc4ef4..b14fb881 100644 --- a/alembic/testing/__init__.py +++ b/alembic/testing/__init__.py @@ -2,6 +2,8 @@ from .fixtures import TestBase from .assertions import eq_, ne_, is_, assert_raises_message, \ eq_ignore_whitespace, assert_raises +from .util import provide_metadata + from alembic import util diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index a2c02397..34e5e998 100644 --- a/docs/build/changelog.rst +++ b/docs/build/changelog.rst @@ -5,6 +5,22 @@ Changelog .. changelog:: :version: 0.7.0 + .. change:: + :tags: bug, autogenerate, postgresql + :tickets: 73 + + Added a routine by which the Postgresql Alembic dialect inspects + the server default of INTEGER/BIGINT columns as they are reflected + during autogenerate for the pattern ``nextval(...)`` containing + a potential sequence name, then queries ``pg_catalog`` to see if this + sequence is "owned" by the column being reflected; if so, it assumes + this is a SERIAL or BIGSERIAL column and the server default is + omitted from the column reflection as well as any kind of + server_default comparison or rendering, along with an INFO message + in the logs indicating this has taken place. This allows SERIAL/BIGSERIAL + columns to keep the SEQUENCE from being unnecessarily present within + the autogenerate operation. + .. change:: :tags: bug, autogenerate :tickets: 197, 64, 196 diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 36b3e498..83131662 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -1,17 +1,18 @@ from sqlalchemy import DateTime, MetaData, Table, Column, text, Integer, \ - String, Interval + String, Interval, Sequence, Numeric, inspect, BigInteger from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.engine.reflection import Inspector from alembic.operations import Operations from sqlalchemy.sql import table, column -from alembic.autogenerate.compare import _compare_server_default +from alembic.autogenerate.compare import \ + _compare_server_default, _compare_tables, _render_server_default_for_compare from alembic import command, util from alembic.migration import MigrationContext from alembic.script import ScriptDirectory -from alembic.testing import eq_ +from alembic.testing import eq_, provide_metadata from alembic.testing.env import staging_env, clear_staging_env, \ _no_sql_testing_config, write_script from alembic.testing.fixtures import capture_context_buffer @@ -310,3 +311,102 @@ class PostgresqlDefaultCompareTest(TestBase): assert not self._compare_default( t1, t2, t2.c.id, "" ) + + +class PostgresqlDetectSerialTest(TestBase): + __only_on__ = 'postgresql' + + @classmethod + def setup_class(cls): + cls.bind = config.db + staging_env() + context = MigrationContext.configure( + connection=cls.bind.connect(), + opts={ + 'compare_type': True, + 'compare_server_default': True + } + ) + connection = context.bind + cls.autogen_context = { + 'imports': set(), + 'connection': connection, + 'dialect': connection.dialect, + 'context': context, + 'opts': { + 'compare_type': True, + 'compare_server_default': True, + 'alembic_module_prefix': 'op.', + 'sqlalchemy_module_prefix': 'sa.', + } + } + + @classmethod + def teardown_class(cls): + clear_staging_env() + + @provide_metadata + def _expect_default(self, c_expected, col, seq=None): + Table('t', self.metadata, col) + + if seq: + seq._set_metadata(self.metadata) + self.metadata.create_all(config.db) + + insp = inspect(config.db) + diffs = [] + _compare_tables( + set([(None, 't')]), set([]), + [], + insp, self.metadata, diffs, self.autogen_context) + tab = diffs[0][1] + eq_(_render_server_default_for_compare( + tab.c.x.server_default, tab.c.x, self.autogen_context), + c_expected) + + insp = inspect(config.db) + diffs = [] + m2 = MetaData() + Table('t', m2, Column('x', BigInteger())) + _compare_tables( + set([(None, 't')]), set([(None, 't')]), + [], + insp, m2, diffs, self.autogen_context) + server_default = diffs[0][0][4]['existing_server_default'] + eq_(_render_server_default_for_compare( + server_default, tab.c.x, self.autogen_context), + c_expected) + + def test_serial(self): + self._expect_default( + None, + Column('x', Integer, primary_key=True) + ) + + def test_separate_seq(self): + seq = Sequence("x_id_seq") + self._expect_default( + "nextval('x_id_seq'::regclass)", + Column( + 'x', Integer, + server_default=seq.next_value(), primary_key=True), + seq + ) + + def test_numeric(self): + seq = Sequence("x_id_seq") + self._expect_default( + "nextval('x_id_seq'::regclass)", + Column( + 'x', Numeric(8, 2), server_default=seq.next_value(), + primary_key=True), + seq + ) + + def test_no_default(self): + self._expect_default( + None, + Column('x', Integer, autoincrement=False, primary_key=True) + ) + +