Skip to content

fix PRD: Classify PG18 RESTRICT (23001) as ForeignKeyViolationError #306

Description

@0x054

Origin: #306 grill (2026-08-15), owner-approved. Downstream: Pinch's delete-path backstop currently accepts either ForeignKeyViolationError or OperationalError (syn54x/pinch-backend@d033032) and flips to ForeignKeyViolationError alone once this lands.

Problem Statement

Deleting a parent that still has children under ForeignKey(on_delete="RESTRICT") is a foreign-key rejection. On PostgreSQL 17 it raises ForeignKeyViolationError. On PostgreSQL 18 the same delete raises OperationalError, so except ForeignKeyViolationError misses it.

Pinch found this because CI ran Postgres 17 and dev ran 18.4. Inserting a dangling FK still classifies correctly on both majors — only RESTRICT on delete/update of the referenced row broke.

Postgres 18 emits SQLSTATE 23001 (restrict_violation) for that case; 17 emitted 23503 (foreign_key_violation). Ferro asks sqlx kind(), and sqlx (0.8 and latest 0.9) only maps 23503. 23001 becomes OtherOperationalError. Ferro is not matching message text. The message change (violates RESTRICT setting of…) is the symptom.

Solution

A RESTRICT parent-delete (or parent-key update) raises ForeignKeyViolationError on every supported backend and every supported Postgres major. sqlstate is the raw driver code: 23503 on PG17, 23001 on PG18, SQLite's extended result code on SQLite. Callers catch the type, not a code or a message.

Ferro owns the integrity-code table that decides the type. sqlx kind() remains the fallback for SQLite and unknown codes. sqlx is not bumped.

User Stories

  1. As an application developer, I want await parent.delete() under on_delete="RESTRICT" to raise ForeignKeyViolationError on PostgreSQL 18, so that the same except I wrote for PostgreSQL 17 still works.
  2. As an application developer, I want await Parent.where(...).delete() to raise the same type as instance.delete(), so that I do not have two catch stories for one constraint.
  3. As an application developer, I want inserting a child with a missing parent to keep raising ForeignKeyViolationError with sqlstate == "23503" on Postgres, so that the save-path contract does not move.
  4. As an application developer, I want except IntegrityError to still catch a RESTRICT parent-delete, so that broad integrity handlers keep working.
  5. As an application developer, I want except OperationalError to stop matching a RESTRICT parent-delete on PostgreSQL 18, so that I do not treat a constraint rejection as an environment failure.
  6. As an application developer, I want exc.sqlstate on that delete to be the code the server sent ("23001" on PG18, "23503" on PG17), so that logs and support diffs match psql / the driver.
  7. As an application developer, I want exc.sqlstate to be present on the postgres delete path (not None), so that I can record the backend code even when I only catch the type.
  8. As an application developer, I want exc.constraint populated on Postgres for a RESTRICT delete, so that I can name the violated FK in an API error.
  9. As an application developer, I want exc.driver_message populated, so that I can log the server text without parsing Ferro's wrapper message.
  10. As an application developer, I want SQLite RESTRICT parent-delete to keep raising ForeignKeyViolationError, so that the matrix backends agree on type.
  11. As an application developer, I want ON UPDATE RESTRICT (changing a referenced key that still has children) to raise ForeignKeyViolationError as well, so that I do not special-case the DML verb.
  12. As an application developer, I want ON DELETE NO ACTION parent-delete to keep raising ForeignKeyViolationError with sqlstate == "23503" on Postgres, so that the PG18 split does not change the NO ACTION contract.
  13. As an application developer, I want ON DELETE CASCADE parent-delete to succeed and not raise, so that classification changes do not invent errors on the default action.
  14. As an application developer using Pinch, I want to drop the dual-class backstop and catch only ForeignKeyViolationError, so that CI-on-17 / dev-on-18 stops being a heisenbug.
  15. As an application developer, I want unique / not-null / check violations to keep their current subclasses and codes, so that this fix does not reshuffle the rest of the tree.
  16. As an application developer, I want an unknown database SQLSTATE to keep raising OperationalError with that code attached, so that Ferro does not guess a subclass.
  17. As an application developer, I want a unique-violation save() to keep raising UniqueViolationError (23505 on Postgres), so that the new table does not steal that mapping.
  18. As an application developer, I want a not-null bulk update to keep raising NotNullViolationError (23502 on Postgres).
  19. As an application developer, I want a check-constraint update to keep raising CheckViolationError (23514 on Postgres).
  20. As an application developer, I want to keep catching by type and never by driver message text, so that the next Postgres wording change cannot break my handlers.
  21. As an application developer reading the exceptions docs, I want to know that ForeignKeyViolationError covers RESTRICT and that PG18 reports 23001, so that I do not assume sqlstate is always 23503.
  22. As a library maintainer, I want a cargo unit test that maps code 23001 to ForeignKeyViolationError without a live Postgres 18, so that CI on postgres:17 cannot green-wash this hole again.
  23. As a library maintainer, I want the live RESTRICT-delete test to accept sqlstate in {"23503", "23001"} on Postgres, so that the same test is honest on 17 and 18.
  24. As a library maintainer, I want the live test to assert sqlstate is not None on Postgres, so that a dropped code fails the suite instead of silently becoming None.
  25. As a library maintainer, I want sqlx to stay on the current 0.8 line, so that this bug is not blocked on a 0.9 migration that still does not map 23001.
  26. As a library maintainer, I want 23000 (generic integrity) and 23P01 (exclusion) to keep today's OperationalError behavior, so that this PR does not invent new exception types.
  27. As a library maintainer, I want the classifier to stay a pure function of (kind, code) (plus the non-database sqlx variants), so that the mapping table is unit-testable without the GIL or a database.
  28. As a contributor adding a future integrity SQLSTATE, I want one table to edit and one cargo test to add, so that we do not wait on sqlx the next time Postgres splits a code.

Implementation Decisions

  • One mapper still converts every engine sqlx::Error into a Ferro exception. Classification of database errors becomes: if the driver code is a Ferro-owned integrity code, that code wins; otherwise sqlx kind() wins; non-database sqlx variants keep today's mapping (OperationalError / InterfaceError / DataError).
  • Ferro-owned integrity codes for this PR:
    • 23001, 23503ForeignKeyViolationError
    • 23505UniqueViolationError
    • 23502NotNullViolationError
    • 23514CheckViolationError
  • sqlstate remains the raw DatabaseError::code() string. Do not rewrite 23001 to 23503.
  • ForeignKeyViolationError is the public type for RESTRICT. No RestrictViolationError.
  • sqlx is not upgraded. Latest 0.9.0 and main still leave 23001 as ErrorKind::Other.
  • CI stays on postgres:17. The 23001 pin is the cargo unit test, not a new CI service.
  • Docs for the exception tree note that RESTRICT is this class and that PG18 reports 23001.
  • CHANGELOG.md is not edited by hand (I-10).
  • No ADR. The mapping is reversible and the surprising part belongs in docs/solutions/ as an issue note (PG18 23001 vs 23503; sqlx kind() lag), not a decision record.

Testing Decisions

Good tests assert exception type and structured attributes (sqlstate, constraint when the backend reports it). They never match driver message text (FF-A exit-gate).

Two existing seams — no new ones:

  1. Classifier (Rust unit). Extend the pure classifier tests that already pin exception_name_for for non-database sqlx variants. Add a (kind, code) → exception name table pin, including 23001ForeignKeyViolationError and the existing integrity codes. This is the PostgreSQL 18 pin. No live PG18.

  2. Public delete path (pytest, backend_matrix). Same module and style as the existing INSERT dangling-FK test. Models with ForeignKey(on_delete="RESTRICT"); persist parent + child; instance.delete() and Query.delete() on the parent both raise ForeignKeyViolationError (and IntegrityError). On postgres: sqlstate in {"23503", "23001"} and sqlstate is not None; constraint is not None. On SQLite: type only (result code differs). The existing INSERT test stays and keeps asserting 23503 on postgres.

instance.delete() and Query.delete() are the same Rust execute helper today (delete_filtered); both public APIs are still tested because that is the contract callers catch on. delete_record is an internal FFI and is not a third live case.

Prior art: tests/test_exception_mapping.py (test_foreign_key_violation_is_typed and siblings); src/errors.rs cargo tests for exception_name_for.

Out of Scope

  • New exception type for RESTRICT.
  • Mapping 23000 (generic integrity) or 23P01 (exclusion); they stay OperationalError.
  • Adding postgres:18 to CI.
  • Upgrading sqlx.
  • Changing save-path classification (already correct on both majors).
  • Normalizing or forging sqlstate.
  • Message-text matching in production or tests.
  • ON UPDATE live coverage beyond the classifier (same SQLSTATE, same mapper). A live UPDATE case is not required to close this.

Further Notes

  • Default on_delete is CASCADE. The live test must set RESTRICT explicitly or it will not raise.
  • ON DELETE NO ACTION stays 23503 on PG18; the bug is RESTRICT-only.
  • If the live postgres delete path actually yields sqlstate is None, that is a second failing assertion in this same PR (propagation), not a follow-up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions