]> git.ipfire.org Git - thirdparty/sqlalchemy/alembic.git/commitdiff
[Fixes #1671] Passthrough `dialect_kwargs` to `ops.create_foreign_key()`
authorJustin Malin <justin@joincandidhealth.com>
Thu, 12 Jun 2025 19:56:36 +0000 (15:56 -0400)
committersqla-tester <sqla-tester@sqlalchemy.org>
Thu, 12 Jun 2025 19:56:36 +0000 (15:56 -0400)
<!-- Provide a general summary of your proposed changes in the Title field above -->

### Description
<!-- Describe your changes in detail -->
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
<!-- go over following points. check them with an `x` if they do apply, (they turn into clickable checkboxes once the PR is submitted, so no need to do everything at once)

-->

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: #<issue number>` 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: #<issue number>` 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

alembic/autogenerate/render.py
docs/build/unreleased/1672.rst [new file with mode: 0644]
tests/test_autogen_render.py
tests/test_postgresql.py

index 4bad437a263397908cf0c283d14d1a6684f2ac85..bd20ced8da6c10293ab72d6e69b0da31333f5597 100644 (file)
@@ -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 (file)
index 0000000..0c636bb
--- /dev/null
@@ -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.
index 55e802307951d08868577fababbcf93f04b595ed..c098aca4e561cdf0bde9119834396cd46d4ca4f5 100644 (file)
@@ -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
index 44f422dd52e8fb61cdacdf82bae5e7304b37deb0..476da5f15ed804df75049445c804783bac40a807 100644 (file)
@@ -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")