Skip to content

Commit 50667ae

Browse files
zzzeekGerrit Code Review
authored andcommitted
Merge "[Fixes #1671] Passthrough dialect_kwargs to ops.create_foreign_key()" into main
2 parents 4ff5cf7 + 229c9f8 commit 50667ae

File tree

4 files changed

+55
-7
lines changed

4 files changed

+55
-7
lines changed

alembic/autogenerate/render.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from sqlalchemy import schema as sa_schema
1919
from sqlalchemy import sql
2020
from sqlalchemy import types as sqltypes
21+
from sqlalchemy.sql.base import _DialectArgView
2122
from sqlalchemy.sql.elements import conv
2223
from sqlalchemy.sql.elements import Label
2324
from sqlalchemy.sql.elements import quoted_name
@@ -31,7 +32,6 @@
3132

3233
from sqlalchemy import Computed
3334
from sqlalchemy import Identity
34-
from sqlalchemy.sql.base import DialectKWArgs
3535
from sqlalchemy.sql.elements import ColumnElement
3636
from sqlalchemy.sql.elements import TextClause
3737
from sqlalchemy.sql.schema import CheckConstraint
@@ -304,11 +304,11 @@ def _drop_table(autogen_context: AutogenContext, op: ops.DropTableOp) -> str:
304304

305305

306306
def _render_dialect_kwargs_items(
307-
autogen_context: AutogenContext, item: DialectKWArgs
307+
autogen_context: AutogenContext, dialect_kwargs: _DialectArgView
308308
) -> list[str]:
309309
return [
310310
f"{key}={_render_potential_expr(val, autogen_context)}"
311-
for key, val in item.dialect_kwargs.items()
311+
for key, val in dialect_kwargs.items()
312312
]
313313

314314

@@ -331,7 +331,7 @@ def _add_index(autogen_context: AutogenContext, op: ops.CreateIndexOp) -> str:
331331

332332
assert index.table is not None
333333

334-
opts = _render_dialect_kwargs_items(autogen_context, index)
334+
opts = _render_dialect_kwargs_items(autogen_context, index.dialect_kwargs)
335335
if op.if_not_exists is not None:
336336
opts.append("if_not_exists=%r" % bool(op.if_not_exists))
337337
text = tmpl % {
@@ -365,7 +365,7 @@ def _drop_index(autogen_context: AutogenContext, op: ops.DropIndexOp) -> str:
365365
"%(prefix)sdrop_index(%(name)r, "
366366
"table_name=%(table_name)r%(schema)s%(kwargs)s)"
367367
)
368-
opts = _render_dialect_kwargs_items(autogen_context, index)
368+
opts = _render_dialect_kwargs_items(autogen_context, index.dialect_kwargs)
369369
if op.if_exists is not None:
370370
opts.append("if_exists=%r" % bool(op.if_exists))
371371
text = tmpl % {
@@ -389,6 +389,7 @@ def _add_unique_constraint(
389389
def _add_fk_constraint(
390390
autogen_context: AutogenContext, op: ops.CreateForeignKeyOp
391391
) -> str:
392+
constraint = op.to_constraint()
392393
args = [repr(_render_gen_name(autogen_context, op.constraint_name))]
393394
if not autogen_context._has_batch:
394395
args.append(repr(_ident(op.source_table)))
@@ -418,9 +419,16 @@ def _add_fk_constraint(
418419
if value is not None:
419420
args.append("%s=%r" % (k, value))
420421

421-
return "%(prefix)screate_foreign_key(%(args)s)" % {
422+
dialect_kwargs = _render_dialect_kwargs_items(
423+
autogen_context, constraint.dialect_kwargs
424+
)
425+
426+
return "%(prefix)screate_foreign_key(%(args)s%(dialect_kwargs)s)" % {
422427
"prefix": _alembic_autogenerate_prefix(autogen_context),
423428
"args": ", ".join(args),
429+
"dialect_kwargs": (
430+
", " + ", ".join(dialect_kwargs) if dialect_kwargs else ""
431+
),
424432
}
425433

426434

@@ -664,7 +672,9 @@ def _uq_constraint(
664672
opts.append(
665673
("name", _render_gen_name(autogen_context, constraint.name))
666674
)
667-
dialect_options = _render_dialect_kwargs_items(autogen_context, constraint)
675+
dialect_options = _render_dialect_kwargs_items(
676+
autogen_context, constraint.dialect_kwargs
677+
)
668678

669679
if alter:
670680
args = [repr(_render_gen_name(autogen_context, constraint.name))]

docs/build/unreleased/1672.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
.. change::
2+
:tags: usecase, operations
3+
:tickets: 1671
4+
Fixed an issue where dialect-specific keyword arguments, dialect_kwargs, were
5+
not passed through when using the op.create_foreign_key() operation. This
6+
prevented the use of backend-specific foreign key options, such as
7+
postgresql_not_valid for PostgreSQL constraints. The renderer for
8+
ops.CreateForeignKeyOp now correctly includes these arguments, aligning its
9+
behavior with other constraint operations.
10+
Pull request courtesy of Justin Malin.

tests/test_autogen_render.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,24 @@ def test_render_drop_index_custom_kwarg(self):
314314
"somedialect_foobar='option')",
315315
)
316316

317+
def test_add_fk_constraint__dialect_kwargs(self):
318+
t1 = self.table()
319+
t2 = self.table()
320+
item = ForeignKeyConstraint(
321+
[t1.c.id], [t2.c.id], name="fk", postgresql_not_valid=True
322+
)
323+
fk_obj = ops.CreateForeignKeyOp.from_constraint(item)
324+
325+
eq_ignore_whitespace(
326+
re.sub(
327+
r"u'",
328+
"'",
329+
autogenerate.render_op_text(self.autogen_context, fk_obj),
330+
),
331+
"op.create_foreign_key('fk', 'test', 'test', ['id'], ['id'], "
332+
"postgresql_not_valid=True)",
333+
)
334+
317335
def test_drop_index_batch(self):
318336
"""
319337
autogenerate.render._drop_index

tests/test_postgresql.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,16 @@ def test_create_index_if_not_exists(self):
133133
op.create_index("i", "t", ["c1", "c2"], if_not_exists=True)
134134
context.assert_("CREATE INDEX IF NOT EXISTS i ON t (c1, c2)")
135135

136+
def test_create_fk_postgresql_not_valid(self):
137+
context = op_fixture("postgresql")
138+
op.create_foreign_key(
139+
"i", "t1", "t2", ["c1"], ["c2"], postgresql_not_valid=True
140+
)
141+
context.assert_(
142+
"ALTER TABLE t1 ADD CONSTRAINT i FOREIGN KEY(c1) "
143+
"REFERENCES t2 (c2) NOT VALID"
144+
)
145+
136146
@config.combinations("include_table", "no_table", argnames="include_table")
137147
def test_drop_index_postgresql_concurrently(self, include_table):
138148
context = op_fixture("postgresql")

0 commit comments

Comments
 (0)