Skip to content

Enable SQLite foreign-key enforcement for application connections #63

Description

@Mattsface

Summary

Enable SQLite foreign-key enforcement for every application-created SQLite connection.

The schema already declares foreign keys for Player persistence, including:

  • player_seasons.player_id -> players.player_id
  • player_season_hitting.player_id -> players.player_id

But SQLite does not enforce declared foreign keys unless PRAGMA foreign_keys = ON is enabled per connection. The current build_engine() only sets check_same_thread=False, so the database can currently accept orphan child rows even though the ORM/schema declares referential integrity.

Goal

Make the SQLite runtime behavior match the declared schema: invalid child rows must be rejected, while all existing valid ingestion and migration paths continue to work.

Scope

Update the database engine construction so every SQLite connection created through the application has foreign-key enforcement enabled.

Prefer the standard SQLAlchemy connection-event approach or an equally focused engine-level solution.

The behavior should be SQLite-specific and must not alter non-SQLite database URLs.

Required behavior

For an engine returned by:

build_engine("sqlite:///...")

a newly checked-out connection should report:

PRAGMA foreign_keys;
-- 1

and referential integrity violations should fail at the database boundary.

Examples that must be rejected:

  • inserting a player_seasons row whose player_id does not exist in players;
  • inserting a player_season_hitting row whose player_id does not exist in players.

Valid application persistence order must continue to work.

Migration compatibility

Verify a fresh SQLite database can still run:

poetry run alembic upgrade head

with foreign-key enforcement enabled.

Do not assume that declared constraints being present in migration metadata proves runtime enforcement.

If any migration genuinely depends on temporarily disabling FK enforcement, handle that explicitly and narrowly rather than globally weakening normal application connections.

Tests

Cover at minimum:

  • build_engine() enables PRAGMA foreign_keys = 1 for SQLite;
  • an orphan player_seasons insert fails;
  • an orphan player_season_hitting insert fails;
  • valid Player identity -> catalog -> hitting persistence succeeds;
  • a fresh migrated database works with FK enforcement enabled;
  • existing ingestion/repository tests continue to pass;
  • non-SQLite engine construction is not coupled to SQLite PRAGMA behavior.

Prefer testing actual database behavior rather than only asserting that an event listener exists.

Preserve

  • SQLite as the current database.
  • Existing SQLAlchemy session ownership and transaction boundaries.
  • Existing ingestion ordering.
  • Alembic as schema migration authority.
  • DB-only browser rendering.
  • Current Player schema and feature behavior.

Out of scope

  • PostgreSQL migration.
  • Cascades or deletion-policy redesign unless required to preserve existing semantics.
  • New repository abstractions.
  • Player counting-stat validation; track that separately.
  • Broad engine/session refactors.

Completion criteria

  • Every normal application SQLite connection has FK enforcement enabled.
  • Orphan Player child records are rejected by SQLite.
  • Fresh migrations and valid Player ingestion still succeed.
  • Full tests and Ruff checks pass.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions