Skip to content

feat(migrate): drop orphaned ck_* under migrate_destructive - #355

Merged
0x054 merged 1 commit into
feat/table-checksfrom
feat/table-checks-345
Aug 18, 2026
Merged

feat(migrate): drop orphaned ck_* under migrate_destructive#355
0x054 merged 1 commit into
feat/table-checksfrom
feat/table-checks-345

Conversation

@0x054

@0x054 0x054 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

A ferro-owned ck_* that is live but gone from the model (removed Check(...), or db_check=True cleared) stays under connect(..., migrate_updates=True) and Ferro warns naming it — leftover CHECKs keep rejecting rows the model now allows, so silence is wrong here. On Postgres, connect(..., migrate_destructive=True) drops that named constraint. Destructive implies updates.

class Transfer(Model):
    __ferro_checks__ = (
        Check(
            "at_most_one_outflow",
            lambda transfer: (transfer.outflow_transaction_id == None)
            | (transfer.outflow_activity_id == None),
        ),
    )
    ...

Remove the Check(...) and reconnect with migrate_updates=True: ck_transfer_at_most_one_outflow is still there, and the warning names it. Reconnect with migrate_destructive=True and Postgres drops it. A hand-created CHECK whose name does not start with ck_ survives both flags.

SQLite warns with the constraint name and skips the in-place rewrite (ADR-0014). Alembic autogenerate always proposes the runtime's DROP CONSTRAINT for an orphaned ferro name — the destructive gate is connect-time safety only (ADR-0013 / ADR-0011: parity is the SQL, not the flag).

Closes #345. Part of #339 (stays open). Blocked-by #344 is already on this branch.

Test plan

  • cargo test -p ferro-ddl-lowering extra_check / render_check_drop (live-order extras, empty extras, warning text, Postgres DROP, SQLite skip, set-difference-only)
  • cargo test -p ferro-migrate plan_check_drops / emit_sql_with_ir_drop_check
  • cargo test -p ferro-schema-ir -p ferro-ddl-lowering -p ferro-migrate
  • cargo test --no-default-features --features testing
  • FERRO_POSTGRES_URL=… uv run pytest tests/test_table_check_orphans.py tests/test_table_check_rebuild.py tests/test_table_check_reconcile.py --db-backends=sqlite,postgres (52 passed)
  • CI on this PR

I-9

Made with Cursor

Leftover ferro-owned CHECKs keep rejecting rows the model now allows, so
migrate_updates warns by name and only migrate_destructive (Postgres)
removes them. Alembic autogenerate always proposes the same DROP.

Co-authored-by: Cursor <cursoragent@cursor.com>
@0x054

0x054 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Gate: clear

Read the full diff. AC for #345 is met.

  • migrate_updates leaves the leftover ck_* and warns naming it (extra_check_names_warning)
  • migrate_destructive drops it on Postgres (render_check_drop); SQLite warn-skip, no rewrite
  • Clearing db_check=True follows the same warn / destructive-drop rule
  • User-owned non-ck_* CHECKs are never extras (caller filters ferro_owned / ck_*; extra_check_names is set-difference only)
  • Alembic _plan_check_drop has no destructive gate; statements match runtime (I-1)
  • Drop ops are not stuffed through the DropIndex retain-filter, so the leftover warning survives a non-destructive plan

Merging into feat/table-checks once CI is green. Does not close parent #339.

@0x054
0x054 merged commit ca40f36 into feat/table-checks Aug 18, 2026
7 checks passed
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.

1 participant