The test database that lied

In April, a user deleted a workspace in production and got a 500. The query behind that delete had passed four rounds of code review and a fully green CI run, and it was still SQL that PostgreSQL refuses to execute. Nothing was wrong with the tests. The problem was who they were asking: my test suite runs on SQLite, production runs on Postgres, and the two disagree about the exact clause that crashed.

A green suite proves your SQL is acceptable to the database you tested against, nothing more. When that isn’t the database you deploy to, the suite can lie to you with a straight face. This is the story of one such lie: what crashed, why four reviews couldn’t see it, and where dialect-sensitive SQL gets checked now.

Postgres won’t lock a row that doesn’t exist

The bug came in with a change that added per-tier workspace caps and a non-deletable Home workspace. A cap check has a classic race: two concurrent create requests can each count ten workspaces, each decide the cap allows one more, and both insert. The standard fix is SELECT ... FOR UPDATE, a row-level lock that makes the second transaction wait until the first commits. I wanted the count and the lock in one round-trip, so I attached the lock directly to the aggregate:

# services/workspace_limits.py: enforce_workspace_create_limit (pre-fix)
counts_stmt = with_for_update_if_supported(
    select(
        func.count().label("total_active"),
        func.count().filter(Workspace.is_default.is_(False)).label("non_default"),
    ).where(owner_filter),
    of=Workspace,
)
row = (await session.execute(counts_stmt)).one()

On Postgres this renders as SELECT count(*) … FROM workspaces … FOR UPDATE OF workspaces, and asyncpg, the Postgres driver, answers with FeatureNotSupportedError: FOR UPDATE is not allowed with aggregate function. The refusal is principled. FOR UPDATE has to know which rows to lock, and an aggregate like count() returns a single computed row that corresponds to no table row. There is nothing to lock.

Two call sites shipped with this shape: enforce_workspace_create_limit in services/workspace_limits.py and soft_delete_workspace in services/workspace.py. Between them, every delete of a non-Home workspace and every second-workspace create was a guaranteed 500. The failure sat latent from the day that change merged until a user tried the delete.

SQLite never saw the lock clause at all

So why was every test green? The first layer is SQLite’s temperament. Handed FOR UPDATE, the test database (sqlite+aiosqlite:///:memory:) ignores the clause instead of rejecting it, so a lock-shape bug and a working query look identical to the suite.

The second layer is my own helper, and it’s the worse one. with_for_update_if_supported exists to keep service code dialect-free: attach the lock on Postgres, skip it on a database where it means nothing. On SQLite it early-returns the bare statement, so the lock clause is never even constructed. In tests, the statement object’s _for_update_arg is None.

That closed off the last chance of a failure, because SQLAlchemy’s compiler cannot raise on a clause that doesn’t exist. The tests weren’t running the broken query leniently. They were running a different query, a correct one.

The suite was honestly green: every assertion in it was true, about the wrong database.

Four review rounds reasoned about semantics, but the bug was syntax

The branch went through four rounds of code review before merging, and the fourth round turned up nothing left to fix. The rounds asked good questions: is the lock target right, are the predicates equivalent across the two call sites, can this deadlock. Those are semantic questions, and the code passes all of them. Lock those rows, count them, one round-trip: as a plan, it’s sound.

The failure was a dialect-specific syntax rejection, and semantic reasoning can’t surface one, because the code means the right thing. Whether Postgres accepts a statement is decidable only by Postgres, or at minimum by its dialect compiler. A fifth review round would have caught nothing. The gap wasn’t diligence; it was structural: nobody, human or CI, ever compiled the rendered SQL against the production dialect.

The fix counts in Python, so the lock has rows to hold

The repair swaps “lock plus aggregate in one query” for “lock the rows, count them in application code”:

# enforce_workspace_create_limit (post-fix)
locked_stmt = with_for_update_if_supported(
    select(Workspace.id, Workspace.is_default).where(owner_filter),
    of=Workspace,
)
rows = (await session.execute(locked_stmt)).all()
total_active = len(rows)
non_default = sum(1 for (_id, is_default) in rows if not is_default)

The lock target is unchanged, the predicate is unchanged, and it’s still one round-trip. The only new cost is pulling rows instead of a single count, and the tier cap bounds the result at 11 rows today, so the difference is noise. soft_delete_workspace got the same treatment. A correct fix, and an unsatisfying place to stop, because it repairs two call sites, not the class of bug.

The Postgres compiler now runs inside the test suite

The structural guard is a statement-level assertion, assert_pg_lock_compatible(stmt) in app/tests/services/_lock_assertions.py. It compiles a statement against postgresql.dialect() and rejects FOR UPDATE combined with anything Postgres refuses to lock: aggregates, GROUP BY, HAVING, DISTINCT, set operations, window functions. The tests still execute against SQLite. The lock shape, though, is now judged by the compiler for the database that actually enforces it.

The regression tests for the two call sites monkeypatch db.session.get_dialect_name to return "postgresql". Without that patch, the same trapdoor opens one level down: the helper strips the lock on SQLite, the statement under test carries no FOR UPDATE, and the assertion passes vacuously. The mechanism that hid the bug was one line away from hiding the regression test too.

This is the second production incident from the SQLite-versus-Postgres gap in this project. The first was an advisory-lock deadlock that the tests missed because they mocked session.execute outright. Two occurrences make a class, so the fix ships with a written escalation threshold: if a third one reaches production, I stand up a Postgres-backed test harness for the affected services. Until then, the compile-time assertion is the right altitude for a project this size: cheap, deterministic, and no test container to babysit.

The invariant I wrote down afterward: any clause whose behavior varies by dialect (lock clauses, window functions, set operations, JSON operators, RETURNING, partial indexes) gets compiled against the production dialect somewhere in the test suite, or it doesn’t ship.

Note what the invariant does not claim. Compiling against postgresql.dialect() proves Postgres will accept the statement, not that it behaves correctly under concurrent load. Acceptance and runtime are different layers, and this guard covers only the first. I think the assertion is the right bet, and I hold the doubt alongside it; that’s what the escalation threshold is for. The first two of these bugs bought an assertion. The third buys a real Postgres in CI.