Skip to content

Commit ff71a4f

Browse files
committed
filter out ::regclass for sequences
Fixed issue where PostgreSQL sequence defaults on non-primary key columns were incorrectly detected as changed on every autogenerate run. Server default comparison logic is adjusted to filter out the ``::regclass`` expression added by the server which interferes with the comparison. Fixes: #1507 Change-Id: I3192b2a86b1982dfbc7c85b49d676e1434dafcf5
1 parent fd42cdd commit ff71a4f

3 files changed

Lines changed: 40 additions & 7 deletions

File tree

alembic/ddl/postgresql.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ def compare_server_default(
113113
rendered_metadata_default,
114114
rendered_inspector_default,
115115
):
116+
116117
# don't do defaults for SERIAL columns
117118
if (
118119
metadata_column.primary_key
@@ -122,6 +123,11 @@ def compare_server_default(
122123

123124
conn_col_default = rendered_inspector_default
124125

126+
if conn_col_default and re.match(
127+
r"nextval\('(.+?)'::regclass\)", conn_col_default
128+
):
129+
conn_col_default = conn_col_default.replace("::regclass", "")
130+
125131
defaults_equal = conn_col_default == rendered_metadata_default
126132
if defaults_equal:
127133
return False
@@ -143,6 +149,8 @@ def compare_server_default(
143149
metadata_default = literal_column(metadata_default)
144150

145151
# run a real compare against the server
152+
# TODO: this seems quite a bad idea for a default that's a SQL
153+
# function! SQL functions are not deterministic!
146154
conn = self.connection
147155
assert conn is not None
148156
return not conn.scalar(

docs/build/unreleased/1507.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
.. change::
2+
:tags: bug, postgresql
3+
:tickets: 1507
4+
5+
Fixed issue where PostgreSQL sequence defaults on non-primary key columns
6+
were incorrectly detected as changed on every autogenerate run. Server
7+
default comparison logic is adjusted to filter out the ``::regclass``
8+
expression added by the server which interferes with the comparison.

tests/test_postgresql.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -668,26 +668,32 @@ class PostgresqlDefaultCompareTest(TestBase):
668668
def setup_class(cls):
669669
cls.bind = config.db
670670
staging_env()
671-
cls.migration_context = MigrationContext.configure(
672-
connection=cls.bind.connect(),
673-
opts={"compare_type": True, "compare_server_default": True},
674-
)
675671

676672
def setUp(self):
677673
self.metadata = MetaData()
674+
self.migration_context = MigrationContext.configure(
675+
connection=self.bind.connect(),
676+
opts={"compare_type": True, "compare_server_default": True},
677+
)
678678
self.autogen_context = api.AutogenContext(self.migration_context)
679679

680680
@classmethod
681681
def teardown_class(cls):
682682
clear_staging_env()
683683

684684
def tearDown(self):
685+
self.migration_context.connection.close()
686+
685687
with config.db.begin() as conn:
686688
self.metadata.drop_all(conn)
687689

688690
def _compare_default_roundtrip(
689691
self, type_, orig_default, alternate=None, diff_expected=None
690692
):
693+
# note this only tests compare_server_default including
694+
# postgresql.compare_server_default. it does not run PG
695+
# autogen_column_reflect() which is involved with omitting SERIAL
696+
# columns
691697
diff_expected = (
692698
diff_expected
693699
if diff_expected is not None
@@ -854,6 +860,16 @@ def test_primary_key_skip(self):
854860
)
855861
assert not self._compare_default(t1, t2, t2.c.id, "")
856862

863+
def test_non_pk_sequence(self):
864+
"""test issue #1507"""
865+
sequential_id_seq = Sequence(
866+
"post_sequential_id_seq", metadata=self.metadata, start=1
867+
)
868+
sequential_id_seq.create(self.bind)
869+
self._compare_default_roundtrip(
870+
Integer, sequential_id_seq.next_value()
871+
)
872+
857873

858874
class PostgresqlDetectSerialTest(TestBase):
859875
__only_on__ = "postgresql"
@@ -943,8 +959,9 @@ def test_separate_seq(self, schema):
943959
seq,
944960
)
945961

946-
@testing.combinations((None,), ("test_schema",))
947-
def test_numeric(self, schema):
962+
@testing.combinations((None,), ("test_schema",), argnames="schema")
963+
@testing.variation("use_pk", [True, False])
964+
def test_numeric(self, schema, use_pk):
948965
seq = Sequence("x_id_seq", schema=schema)
949966
seq_name = seq.name if schema is None else f"{schema}.{seq.name}"
950967
self._expect_default(
@@ -953,7 +970,7 @@ def test_numeric(self, schema):
953970
"x",
954971
Numeric(8, 2),
955972
server_default=seq.next_value(),
956-
primary_key=True,
973+
primary_key=bool(use_pk),
957974
),
958975
schema,
959976
seq,

0 commit comments

Comments
 (0)