From: Justin Malin Date: Thu, 12 Jun 2025 19:56:36 +0000 (-0400) Subject: [Fixes #1671] Passthrough `dialect_kwargs` to `ops.create_foreign_key()` X-Git-Tag: rel_1_16_2~4^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=229c9f858d11759abd0746d3bf6f7a6a4e001a50;p=thirdparty%2Fsqlalchemy%2Falembic.git [Fixes #1671] Passthrough `dialect_kwargs` to `ops.create_foreign_key()` ### Description Fixes #1671 SqlAlchemy supports adding dialect kwargs for foreign keys, as does `op.create_foreign_key()`, but the renderer for `ops.CreateForeignKeyOp` does not pass through `dialect_kwargs`. An example of this is `postgresql_not_valid`. ### Checklist This pull request is: - [ ] A documentation / typographical error fix - Good to go, no issue or tests are needed - [x] A short code fix - please include the issue number, and create an issue if none exists, which must include a complete example of the issue. one line code fixes without an issue and demonstration will not be accepted. - Please include: `Fixes: #` in the commit message - please include tests. one line code fixes without tests will not be accepted. - [ ] A new feature implementation - please include the issue number, and create an issue if none exists, which must include a complete example of how the feature would look. - Please include: `Fixes: #` in the commit message - please include tests. **Have a nice day!** Closes: #1672 Pull-request: https://github.com/sqlalchemy/alembic/pull/1672 Pull-request-sha: 246a363617b1a4a5885d2fed32e82f3312b386ef Change-Id: I5dabbc4dc408c13883a6225733d7c38f8daeee38 --- diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py index 4bad437a..bd20ced8 100644 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@ -18,6 +18,7 @@ from mako.pygen import PythonPrinter from sqlalchemy import schema as sa_schema from sqlalchemy import sql from sqlalchemy import types as sqltypes +from sqlalchemy.sql.base import _DialectArgView from sqlalchemy.sql.elements import conv from sqlalchemy.sql.elements import Label from sqlalchemy.sql.elements import quoted_name @@ -31,7 +32,6 @@ if TYPE_CHECKING: from sqlalchemy import Computed from sqlalchemy import Identity - from sqlalchemy.sql.base import DialectKWArgs from sqlalchemy.sql.elements import ColumnElement from sqlalchemy.sql.elements import TextClause from sqlalchemy.sql.schema import CheckConstraint @@ -304,11 +304,11 @@ def _drop_table(autogen_context: AutogenContext, op: ops.DropTableOp) -> str: def _render_dialect_kwargs_items( - autogen_context: AutogenContext, item: DialectKWArgs + autogen_context: AutogenContext, dialect_kwargs: _DialectArgView ) -> list[str]: return [ f"{key}={_render_potential_expr(val, autogen_context)}" - for key, val in item.dialect_kwargs.items() + for key, val in dialect_kwargs.items() ] @@ -331,7 +331,7 @@ def _add_index(autogen_context: AutogenContext, op: ops.CreateIndexOp) -> str: assert index.table is not None - opts = _render_dialect_kwargs_items(autogen_context, index) + opts = _render_dialect_kwargs_items(autogen_context, index.dialect_kwargs) if op.if_not_exists is not None: opts.append("if_not_exists=%r" % bool(op.if_not_exists)) text = tmpl % { @@ -365,7 +365,7 @@ def _drop_index(autogen_context: AutogenContext, op: ops.DropIndexOp) -> str: "%(prefix)sdrop_index(%(name)r, " "table_name=%(table_name)r%(schema)s%(kwargs)s)" ) - opts = _render_dialect_kwargs_items(autogen_context, index) + opts = _render_dialect_kwargs_items(autogen_context, index.dialect_kwargs) if op.if_exists is not None: opts.append("if_exists=%r" % bool(op.if_exists)) text = tmpl % { @@ -389,6 +389,7 @@ def _add_unique_constraint( def _add_fk_constraint( autogen_context: AutogenContext, op: ops.CreateForeignKeyOp ) -> str: + constraint = op.to_constraint() args = [repr(_render_gen_name(autogen_context, op.constraint_name))] if not autogen_context._has_batch: args.append(repr(_ident(op.source_table))) @@ -418,9 +419,16 @@ def _add_fk_constraint( if value is not None: args.append("%s=%r" % (k, value)) - return "%(prefix)screate_foreign_key(%(args)s)" % { + dialect_kwargs = _render_dialect_kwargs_items( + autogen_context, constraint.dialect_kwargs + ) + + return "%(prefix)screate_foreign_key(%(args)s%(dialect_kwargs)s)" % { "prefix": _alembic_autogenerate_prefix(autogen_context), "args": ", ".join(args), + "dialect_kwargs": ( + ", " + ", ".join(dialect_kwargs) if dialect_kwargs else "" + ), } @@ -664,7 +672,9 @@ def _uq_constraint( opts.append( ("name", _render_gen_name(autogen_context, constraint.name)) ) - dialect_options = _render_dialect_kwargs_items(autogen_context, constraint) + dialect_options = _render_dialect_kwargs_items( + autogen_context, constraint.dialect_kwargs + ) if alter: args = [repr(_render_gen_name(autogen_context, constraint.name))] diff --git a/docs/build/unreleased/1672.rst b/docs/build/unreleased/1672.rst new file mode 100644 index 00000000..0c636bb8 --- /dev/null +++ b/docs/build/unreleased/1672.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: usecase, operations + :tickets: 1671 + Fixed an issue where dialect-specific keyword arguments, dialect_kwargs, were + not passed through when using the op.create_foreign_key() operation. This + prevented the use of backend-specific foreign key options, such as + postgresql_not_valid for PostgreSQL constraints. The renderer for + ops.CreateForeignKeyOp now correctly includes these arguments, aligning its + behavior with other constraint operations. + Pull request courtesy of Justin Malin. diff --git a/tests/test_autogen_render.py b/tests/test_autogen_render.py index 55e80230..c098aca4 100644 --- a/tests/test_autogen_render.py +++ b/tests/test_autogen_render.py @@ -314,6 +314,24 @@ class AutogenRenderTest(TestBase): "somedialect_foobar='option')", ) + def test_add_fk_constraint__dialect_kwargs(self): + t1 = self.table() + t2 = self.table() + item = ForeignKeyConstraint( + [t1.c.id], [t2.c.id], name="fk", postgresql_not_valid=True + ) + fk_obj = ops.CreateForeignKeyOp.from_constraint(item) + + eq_ignore_whitespace( + re.sub( + r"u'", + "'", + autogenerate.render_op_text(self.autogen_context, fk_obj), + ), + "op.create_foreign_key('fk', 'test', 'test', ['id'], ['id'], " + "postgresql_not_valid=True)", + ) + def test_drop_index_batch(self): """ autogenerate.render._drop_index diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 44f422dd..476da5f1 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -133,6 +133,16 @@ class PostgresqlOpTest(TestBase): op.create_index("i", "t", ["c1", "c2"], if_not_exists=True) context.assert_("CREATE INDEX IF NOT EXISTS i ON t (c1, c2)") + def test_create_fk_postgresql_not_valid(self): + context = op_fixture("postgresql") + op.create_foreign_key( + "i", "t1", "t2", ["c1"], ["c2"], postgresql_not_valid=True + ) + context.assert_( + "ALTER TABLE t1 ADD CONSTRAINT i FOREIGN KEY(c1) " + "REFERENCES t2 (c2) NOT VALID" + ) + @config.combinations("include_table", "no_table", argnames="include_table") def test_drop_index_postgresql_concurrently(self, include_table): context = op_fixture("postgresql")