From 733197e0e794ed6aec3e21542257d6cec5bb7d1f Mon Sep 17 00:00:00 2001 From: d3 Date: Tue, 29 Aug 2023 07:27:04 -0400 Subject: [PATCH] Allow passoing ExcludeConstraint to DropConstraint.from_constraint Added support for ``op.drop_constraint()`` to support PostrgreSQL ``ExcludeConstraint`` objects, as well as other constraint-like objects that may be present in third party dialects, by resolving the ``type_`` parameter to be ``None`` for this case. Autogenerate has also been enhanced to exclude the ``type_`` parameter from rendering within this command when ``type_`` is ``None``. Pull request courtesy David Hills. Fixes: #1300 Closes: #1301 Pull-request: https://github.com/sqlalchemy/alembic/pull/1301 Pull-request-sha: b8643d19ab1930554e1137b64ffea8b8e2c74ec7 Change-Id: I25599d24a8bba455ab1d9b88843d8d84627a72d5 --- alembic/autogenerate/render.py | 29 ++++++++++++++--------------- alembic/operations/ops.py | 2 +- docs/build/unreleased/1300.rst | 12 ++++++++++++ tests/test_op.py | 5 +++++ tests/test_postgresql.py | 24 ++++++++++++++++++++++++ 5 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 docs/build/unreleased/1300.rst diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py index d6cad2c5..3729a486 100644 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@ -398,22 +398,21 @@ def _add_check_constraint(constraint, autogen_context): def _drop_constraint( autogen_context: AutogenContext, op: ops.DropConstraintOp ) -> str: - if autogen_context._has_batch: - template = "%(prefix)sdrop_constraint" "(%(name)r, type_=%(type)r)" - else: - template = ( - "%(prefix)sdrop_constraint" - "(%(name)r, '%(table_name)s'%(schema)s, type_=%(type)r)" - ) + prefix = _alembic_autogenerate_prefix(autogen_context) + name = _render_gen_name(autogen_context, op.constraint_name) + schema = _ident(op.schema) if op.schema else None + type_ = _ident(op.constraint_type) if op.constraint_type else None - text = template % { - "prefix": _alembic_autogenerate_prefix(autogen_context), - "name": _render_gen_name(autogen_context, op.constraint_name), - "table_name": _ident(op.table_name), - "type": op.constraint_type, - "schema": (", schema=%r" % _ident(op.schema)) if op.schema else "", - } - return text + params_strs = [] + params_strs.append(repr(name)) + if not autogen_context._has_batch: + params_strs.append(repr(_ident(op.table_name))) + if schema is not None: + params_strs.append(f"schema={schema!r}") + if type_ is not None: + params_strs.append(f"type_={type_!r}") + + return f"{prefix}drop_constraint({', '.join(params_strs)})" @renderers.dispatch_for(ops.AddColumnOp) diff --git a/alembic/operations/ops.py b/alembic/operations/ops.py index 0b66cd70..68c44eb6 100644 --- a/alembic/operations/ops.py +++ b/alembic/operations/ops.py @@ -171,7 +171,7 @@ def from_constraint(cls, constraint: Constraint) -> DropConstraintOp: sqla_compat.constraint_name_or_none(constraint.name), constraint_table.name, schema=constraint_table.schema, - type_=types[constraint.__visit_name__], + type_=types.get(constraint.__visit_name__), _reverse=AddConstraintOp.from_constraint(constraint), ) diff --git a/docs/build/unreleased/1300.rst b/docs/build/unreleased/1300.rst new file mode 100644 index 00000000..048c3963 --- /dev/null +++ b/docs/build/unreleased/1300.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, operations + :tickets: 1300 + + Added support for ``op.drop_constraint()`` to support PostrgreSQL + ``ExcludeConstraint`` objects, as well as other constraint-like objects + that may be present in third party dialects, by resolving the ``type_`` + parameter to be ``None`` for this case. Autogenerate has also been + enhanced to exclude the ``type_`` parameter from rendering within this + command when ``type_`` is ``None``. Pull request courtesy David Hills. + + diff --git a/tests/test_op.py b/tests/test_op.py index 64632443..403a94aa 100644 --- a/tests/test_op.py +++ b/tests/test_op.py @@ -808,6 +808,11 @@ def test_drop_constraint_type(self): op.drop_constraint("foo_bar_bat", "t1", type_="foreignkey") context.assert_("ALTER TABLE t1 DROP CONSTRAINT foo_bar_bat") + def test_drop_constraint_type_generic(self): + context = op_fixture() + op.drop_constraint("foo_bar_bat", "t1") + context.assert_("ALTER TABLE t1 DROP CONSTRAINT foo_bar_bat") + def test_drop_constraint_legacy_type(self): """#1245""" context = op_fixture() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index f0db6699..99f6e9fc 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -1263,6 +1263,30 @@ def test_inline_exclude_constraint_text(self): "name='TExclID'))", ) + def test_drop_exclude_constraint(self): + """test for #1300""" + + autogen_context = self.autogen_context + + m = MetaData() + t = Table( + "TTable", m, Column("XColumn", String), Column("YColumn", String) + ) + + op_obj = ops.DropConstraintOp.from_constraint( + ExcludeConstraint( + (t.c.XColumn, ">"), + where=t.c.XColumn != 2, + using="gist", + name="t_excl_x", + ) + ) + + eq_ignore_whitespace( + autogenerate.render_op_text(autogen_context, op_obj), + "op.drop_constraint('t_excl_x', 'TTable')", + ) + def test_json_type(self): eq_ignore_whitespace( autogenerate.render._repr_type(JSON(), self.autogen_context),