Downgrading to depends_on migration causes "Requested revision ... overlaps with other requested revisions ..." issue

[saifelse]

Describe the bug
When downgrading to a migration (b1) that is the depends_on of a branched migration (a1), the alembic_version table incorrectly includes both a1 and b1 when it should just have a1. This bug then prevents further upgrades from working, like upgrading to a merge migration (bmerge) that merges a1 and b1.

Expected behavior
Downgrading to b1 should result in a1 being in the alembic_version table. From there, upgrading to bmerge should work as expected.

To Reproduce
Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved.
See also Reporting Bugs on the website.

The following patch can be applied:

diff --git a/tests/test_version_traversal.py b/tests/test_version_traversal.py
index 0628f3f..dcb342a 100644
--- a/tests/test_version_traversal.py
+++ b/tests/test_version_traversal.py
@@ -1176,6 +1176,44 @@ class DependsOnBranchTestFour(MigrationTest):
         )
 
 
+class DependsOnBranchTestFive(MigrationTest):
+    @classmethod
+    def setup_class(cls):
+        """
+        Structure::
+        <base> ->a1 ------+
+                 ^        |
+                 |        +-> bmerge
+                 |        |
+                 +---b1---+
+        """
+        cls.env = env = staging_env()
+        cls.a1 = env.generate_revision("a1", "->a1")
+        cls.b1 = env.generate_revision("b1", "->b1", head="base", depends_on="a1")
+        cls.bmerge = env.generate_revision("bmerge"," bmerge", head=[cls.a1.revision, cls.b1.revision])
+
+    @classmethod
+    def teardown_class(cls):
+        clear_staging_env()
+
+    def test_downgrade_to_depends_on(self):
+        # Upgrade from a1 to b1 just has heads={"b1"}.
+        self._assert_upgrade(self.b1.revision, self.a1.revision, expected=[self.up_(self.b1)], expected_heads={self.b1.revision})
+
+        # Upgrade from b1 to bmerge just has {"bmerge"}
+        self._assert_upgrade(self.bmerge.revision, self.b1.revision, expected=[self.up_(self.bmerge)], expected_heads={self.bmerge.revision})
+
+        # Downgrading from {"bmerge"} to {"a1"} should be back at {"b1"}, but instead we fail our assertion.
+        #   AssertionError: {'b1', 'a1'} != {'b1'}
+        self._assert_downgrade(self.a1.revision, self.bmerge.revision, expected=[self.down_(self.bmerge)], expected_heads={self.b1.revision})
+
+        # TODO: Delete the following - it just illustrates the downstream error. To see this failure mode, comment out the above `_assert_downgrade`.
+        # In actual usage, `alembic downgrade` above would "succeed", but it ends up writing [a1, b1] to alembic_version table, which is corrupt.
+        # And now if we try to now migrate from {'a1, 'b1'} to "bmerge", we hit this confusing crash:
+        #   alembic.util.exc.CommandError: Requested revision b1 overlaps with other requested revisions a1
+        self._assert_upgrade(self.bmerge.revision, [self.b1.revision, self.a1.revision], expected=[self.up_(self.bmerge)], expected_heads={self.bmerge.revision})
+
+
 class DependsOnBranchLabelTest(MigrationTest):
     @classmethod
     def setup_class(cls):

Error

  File "/Users/saif/contrib/alembic/tests/test_version_traversal.py", line 1208, in test_downgrade_to_depends_on
    self._assert_downgrade(self.a1.revision, self.bmerge.revision, expected=[self.down_(self.bmerge)], expected_heads={self.b1.revision})
  File "/Users/saif/contrib/alembic/tests/test_version_traversal.py", line 27, in _assert_downgrade
    eq_(head.heads, expected_heads)
  File "/Users/saif/.pyenv/versions/3.9/envs/alembic/lib/python3.11/site-packages/sqlalchemy/testing/assertions.py", line 283, in eq_
    assert a == b, msg or "%r != %r" % (a, b)
AssertionError: {'b1', 'a1'} != {'b1'}
assert {'a1', 'b1'} == {'b1'}
  Extra items in the left set:
  'a1'
  Full diff:
  - {'b1'}
  + {'b1', 'a1'}

If the failing assertion is commented out, we see that our heads state is corrupt:

  File "/Users/saif/contrib/alembic/tests/test_version_traversal.py", line 1214, in test_downgrade_to_depends_on
    self._assert_upgrade(self.bmerge.revision, [self.b1.revision, self.a1.revision], expected=[self.up_(self.bmerge)], expected_heads={self.bmerge.revision})
  File "/Users/saif/contrib/alembic/tests/test_version_traversal.py", line 30, in _assert_upgrade
    revs = self.env._upgrade_revs(destination, source)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/saif/contrib/alembic/alembic/script/base.py", line 449, in _upgrade_revs
    with self._catch_revision_errors(
  File "/opt/homebrew/Cellar/[email protected]/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/Users/saif/contrib/alembic/alembic/script/base.py", line 285, in _catch_revision_errors
    raise util.CommandError(err.args[0]) from err
alembic.util.exc.CommandError: Requested revision b1 overlaps with other requested revisions a1

Versions.

• OS: macOS 14.1.1
• Python: 3.11.6 (and
• Alembic: main (and 1.10.2)
• SQLAlchemy: 2.0.23 (and 1.3.24)
• Database: N/A
• DBAPI: N/A

Additional context

Have a nice day!

[zzzeek]

hi -

This seems to be a cycle of some kind you are creating, so if you have the resources to provide a pull request to allow this test to pass without breaking any others, we can review. otherwise there are no plans to work on these issues as downgrades are not considered to be production critical by the Alembic project.

[sqla-tester]

Saif Hakim has proposed a fix for this issue in the main branch:

Fix downgrade when normalized down revisions have overlap via depends_on https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5027

[sqla-tester]

Saif Hakim has proposed a fix for this issue in the main branch:

Fix downgrade when normalized down revisions have overlap via depends_on https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5027