Skip to content

Only treat a missing migrations table as no applied migrations - #2382

Open
rhymiz wants to merge 6 commits into
tortoise:developfrom
rhymiz:fix/applied-migrations-errors
Open

rhymiz wants to merge 6 commits into
tortoise:developfrom
rhymiz:fix/applied-migrations-errors

Conversation

@rhymiz

@rhymiz rhymiz commented Oct 24, 2026 •

Copy link
Copy Markdown

Description

MigrationRecorder.applied_migrations() caught every exception from its SELECT and returned []:

try:
    _, rows = await self.connection.execute_query(query)
except Exception:
    return []

This PR adds _table_exists(). It asks the dialect's catalog whether the migrations table exists, using the lookup that matches how the unqualified SELECT resolves the name:

  • sqlite: sqlite_master
  • postgres: to_regclass(...) (follows search_path)
  • mysql: information_schema.tables with table_schema = DATABASE()
  • mssql: OBJECT_ID(?, 'U')
  • oracle: all_tables with owner = SYS_CONTEXT('USERENV', 'CURRENT_SCHEMA')

applied_migrations() now returns [] only when the table is absent, and every other error propagates. Dialects outside these five run the SELECT directly, so their errors propagate too. Checking the catalog avoids depending on each backend's exception translation, which wraps driver errors differently. It also means a failed SELECT no longer breaks an open PostgreSQL transaction.

Motivation and Context

We found this in production. Our PostgreSQL server refused connections with "too many clients already", and applied_migrations() reported no applied migrations. Our startup check then said the database lacked the build's migrations, which pointed at the wrong problem. Authentication failures, timeouts and permission errors were reported the same way. Only a missing migrations table should mean "none applied".

How Has This Been Tested?

New tests in tests/migrations/test_recorder.py:

  • For each dialect (sqlite, postgres, mysql, mssql, oracle), a missing table returns [] without querying the table.
  • For each dialect, a DBConnectionError propagates.
  • An OperationalError on the SELECT propagates when the table exists.
  • Against the real test database, applied_migrations() returns [] both before and after MigrationExecutor.migrate() creates the table.

The error-propagation tests fail on the old recorder and pass with this change.

Results:

  • make test options on sqlite: 2131 passed, 149 skipped, 2 xfailed.
  • tests/migrations and tests/cli on PostgreSQL with asyncpg: 358 passed. With psycopg: 358 passed.
  • ruff format --check, ruff check, mypy and bandit pass.

Not run against real MySQL, MSSQL or Oracle; for those dialects, only the fake-connection tests cover the change.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codspeed

codspeed Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

⚠️ 21 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 24 untouched benchmarks


Comparing rhymiz:fix/applied-migrations-errors (90c05e1) with develop (cdef122)

Open in CodSpeed

Comment thread tortoise/migrations/recorder.py Outdated
except Exception:
if not await self._table_exists():
return []
_, rows = await self.connection.execute_query(query)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhymiz How about:

        if not await self._table_exists():
            return []
        try:
            _, rows = await self.connection.execute_query(query)
        except Exception:
            return []

MigrationRecorder.applied_migrations() caught every exception from its
SELECT and returned an empty list. A refused connection ("too many
clients already"), an authentication failure, a timeout or a permission
error therefore looked like a database with no migrations applied.

Check the dialect's catalog for the migrations table first (sqlite_master,
to_regclass, information_schema.tables, OBJECT_ID, all_tables) and return
an empty list only when it is absent. Every other error now propagates.
@rhymiz
rhymiz force-pushed the fix/applied-migrations-errors branch from 8506df0 to 62aa3b5 Compare October 26, 2026 08:46
@rhymiz

rhymiz commented Oct 26, 2026

Copy link
Copy Markdown
Author

@waketzheng mind taking a look again?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants