feat(migrate): add missing ck_* constraints on migrate_updates - #353
Merged
Conversation
`migrate_updates=True` now reconciles CHECK constraints: a table check or column check that the model declares but the live table is missing is added with `ALTER TABLE … ADD CONSTRAINT` on Postgres. Existing rows must satisfy it — otherwise connect fails and that table's plan rolls back through the per-table transaction. Toggling `Field(db_check=True)` on an existing column now actually creates `ck_<table>_<col>`. The decision (which declared names have no live counterpart) and the rendered DDL are single-sourced in `ferro_ddl_lowering::missing_check_names` / `render_check_addition`. The reconciliation pass consumes them through `ferro_migrate::plan_missing_checks` + the new `MigrationOp::AddCheck`; the Alembic autogenerate comparator consumes the same pair over FFI (`_plan_check_addition`) and executes byte-identical statements (I-1). Comparison is by name only: a live `ck_*` whose body drifted is a rebuild (#344), a live CHECK ferro does not own is never touched, and orphaned `ck_*` are left alone (#345). On SQLite the add is skipped with a warning naming the constraint — adding a table constraint needs a full table rebuild, which is Alembic's batch-mode door (ADR-0014). The reconciliation pass now also skips tables the create pass built in the same run (ADR-0010): they are already exactly the model, so re-diffing could only replay that pass's own backend-limitation warnings. Co-authored-by: Cursor <cursoragent@cursor.com>
The add path is the same shape as enum-label addition: one Rust pair, consumed by auto-migrate and Alembic over FFI. Name it here so a third emitter cannot grow a second missing-check table. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
Gate: clearRead the full diff. AC for #343 is met.
Nit pushed: I-1 item 12 names Merging into |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #343
Slice of #339 (do not close). Base is
feat/table-checks.What a user sees
A model gains an invariant, and the next boot enforces it on the table that is already there:
Same for a column check: flipping
Field(db_check=True)on a column that already exists now actually createsck_<table>_<col>(it used to be create-time only). If existing rows violate the new CHECK, the connect fails withCheckViolationErrorand that table's whole plan rolls back — the column added in the same run is gone too, so the run is safely re-runnable.On SQLite nothing is altered: adding a table constraint needs a full table rebuild, so the pass warns naming the constraint and points at Alembic's batch mode (ADR-0014).
Alembic autogenerate proposes the byte-identical statement, with no
migrate_updatesgate — running autogenerate is itself the request for a diff.How
ferro_ddl_lowering::missing_check_namesdecides which declaredck_*names have no live counterpart;render_check_additionrenders the ADD (a plainALTER TABLE … ADD CONSTRAINTfor a table check; the existing idempotentrender_db_checkDO-block for a column check, single-sourced with the create path). The reconciliation pass consumes them throughferro_migrate::plan_missing_checks+ the newMigrationOp::AddCheck; the Alembic comparator consumes the same pair over FFI (_plan_check_addition).live_table_checks()reports a name plus the backend's rendering of the body. Feeding that rendering into aSchemaCheck/SchemaTableCheckwould create a second body language next to the IR predicate, which I-1 forbids — soplan_table_migrationtakeslive_checks: &[LiveCheck]and the decision stays name-based.plan_missing_checksruns afterplan_from_ir, so a CHECK over a newly added column lands after itsADD COLUMN. A column check on a column being added in this same run is filtered out —emit_add_columnalready emits its DO-block (the same dedupdiff_model_indexesdoes for single-column indexes).db_checktable warned twice).internal_create_tablesreturns the pre-existing table set to make that explicit.Non-goals (untouched)
Body drift of a same-named CHECK is a rebuild (#344) — comparison is by name only. Orphan
ck_*drops are #345.src/ferro/checks.pyis unchanged (#346). Docs are #347. A live CHECK ferro does not own (ferro_owned: false) is never added over or dropped.Test plan
cargo test --workspace --no-default-features --features testing— 367 tests, new pins inferro-ddl-lowering(decision + both renderers + SQLite skip + unknown-nameNone) andferro-migrate(planning, dedup, emission, ordering, loud failure on an undeclared name)uv run pytest --db-backends=sqlite,postgres— 1777 passed, 11 skipped, 1 xfailedtests/test_table_check_reconcile.py(tests/test_table_checks.pyuntouched):migrate_updatesboot; name present inpg_constraint; the invariant then rejects a violating insertdb_check=Trueon a live column addsck_cookie_flavorand rejects an out-of-domain valuemigrate_updates=Falseplans nothing (ADR-0010)Exit steps
Closes #343in this body;featPRD: table-level CHECK constraints (multi-column), auto-migrated #339 left open and uneditedCHANGELOG.mduntouched (I-10)Made with Cursor