From e3d5339d6ee119833863d381c31f8ba59e0cd71d Mon Sep 17 00:00:00 2001 From: Riti Grover Date: Tue, 15 Sep 2026 20:42:06 +0530 Subject: [PATCH 1/2] fix(postgresql): identity column not added when existing_server_default omitted Operations.alter_column defaults existing_server_default to False (its 'not specified' sentinel), but the PostgreSQL identity-column compiler only checked for None to decide whether to render 'ADD GENERATED ... AS IDENTITY'. False fell through into the diff-against-existing-identity branch, computed an empty diff, and rendered an invalid, clause-less ALTER TABLE ... ALTER COLUMN statement. Fixes #1504 --- alembic/ddl/postgresql.py | 5 +++-- docs/build/unreleased/1504.rst | 13 +++++++++++++ tests/test_postgresql.py | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 docs/build/unreleased/1504.rst diff --git a/alembic/ddl/postgresql.py b/alembic/ddl/postgresql.py index 46b17fa2..559b9971 100644 --- a/alembic/ddl/postgresql.py +++ b/alembic/ddl/postgresql.py @@ -569,8 +569,9 @@ def visit_identity_column( # drop identity text += "DROP IDENTITY" return text - elif element.existing_server_default is None: - # add identity options + elif element.existing_server_default in (None, False): + # no known existing default (False is the "not specified" + # sentinel used by Operations.alter_column) - add identity options text += "ADD " text += compiler.visit_identity_column(element.default) return text diff --git a/docs/build/unreleased/1504.rst b/docs/build/unreleased/1504.rst new file mode 100644 index 00000000..d71c59a7 --- /dev/null +++ b/docs/build/unreleased/1504.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, postgresql + :tickets: 1504 + + Fixed bug where :meth:`.Operations.alter_column` against a PostgreSQL + identity column would render an incomplete, invalid ``ALTER TABLE ... + ALTER COLUMN`` statement with no clause at all when + ``existing_server_default`` was left at its default of not being + specified. The identity-column DDL compiler only recognized an explicit + ``None`` as "no existing default", not the sentinel value used + internally by :meth:`.Operations.alter_column` when the argument is + omitted, so the "add identity" branch was skipped and no options were + ever rendered. diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 5a9f7c70..b497383d 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -449,6 +449,24 @@ def test_remove_identity_from_column(self): "ALTER TABLE t1 ALTER COLUMN some_column DROP IDENTITY" ) + def test_add_identity_to_column_no_existing_server_default(self): + """existing_server_default left at its default (``False``, meaning + "not specified") must be treated the same as an explicit ``None``, + or the identity is never actually added. + + Regression test for https://github.com/sqlalchemy/alembic/issues/1504 + """ + context = op_fixture("postgresql") + op.alter_column( + "t1", + "some_column", + server_default=Identity(on_null=True), + ) + context.assert_( + "ALTER TABLE t1 ALTER COLUMN some_column ADD " + "GENERATED BY DEFAULT AS IDENTITY" + ) + @combinations( ({}, dict(always=True), "SET GENERATED ALWAYS"), ( From 6e594213ba55b9ae9c5a08eca3dfe62474f4f284 Mon Sep 17 00:00:00 2001 From: Riti Grover Date: Wed, 16 Sep 2026 10:26:24 +0530 Subject: [PATCH 2/2] test: fold identity regression case into existing matrix Per review feedback: reuse test_add_identity_to_column instead of a separate test function, and drop Identity(on_null=True) from the regression case since its rendering isn't consistent across all tested SQLAlchemy versions. The {} (no options) case already covers the regression (existing_server_default omitted vs explicit None) without relying on a non-portable option. --- tests/test_postgresql.py | 46 +++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index b497383d..4508c57c 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -415,21 +415,31 @@ def test_add_column_identity(self, kw, text): ) @combinations( - ({}, None), - (dict(always=True), None), + ({}, None, True), + ({}, None, False), + (dict(always=True), None, True), ( dict(start=3, increment=33, maxvalue=99, cycle=True), "INCREMENT BY 33 START WITH 3 MAXVALUE 99 CYCLE", + True, ), ) - def test_add_identity_to_column(self, kw, text): + def test_add_identity_to_column( + self, kw, text, pass_existing_server_default + ): context = op_fixture("postgresql") - op.alter_column( - "t1", - "some_column", - server_default=Identity(**kw), - existing_server_default=None, - ) + alter_column_kw = { + "server_default": Identity(**kw), + } + if pass_existing_server_default: + alter_column_kw["existing_server_default"] = None + # else: leave existing_server_default at its default, which is the + # ``False`` "not specified" sentinel, not ``None``. It must be + # treated the same as an explicit ``None``, or the identity is + # never actually added. + # Regression test for + # https://github.com/sqlalchemy/alembic/issues/1504 + op.alter_column("t1", "some_column", **alter_column_kw) qualification = "ALWAYS" if kw.get("always", False) else "BY DEFAULT" options = " (%s)" % text if text else "" context.assert_( @@ -449,24 +459,6 @@ def test_remove_identity_from_column(self): "ALTER TABLE t1 ALTER COLUMN some_column DROP IDENTITY" ) - def test_add_identity_to_column_no_existing_server_default(self): - """existing_server_default left at its default (``False``, meaning - "not specified") must be treated the same as an explicit ``None``, - or the identity is never actually added. - - Regression test for https://github.com/sqlalchemy/alembic/issues/1504 - """ - context = op_fixture("postgresql") - op.alter_column( - "t1", - "some_column", - server_default=Identity(on_null=True), - ) - context.assert_( - "ALTER TABLE t1 ALTER COLUMN some_column ADD " - "GENERATED BY DEFAULT AS IDENTITY" - ) - @combinations( ({}, dict(always=True), "SET GENERATED ALWAYS"), (