Add CASCADE to `logged_model` tables `experiment_id` foreign keys by harupy · Pull Request #20185 · mlflow/mlflow

harupy · 2026-01-21T11:17:34Z

Related Issues/PRs

Resolve #20184

What changes are proposed in this pull request?

Add ON DELETE CASCADE to the experiment_id foreign key constraints in logged_model_params, logged_model_tags, and logged_model_metrics tables. This fixes GC failing when attempting to delete experiments that have associated logged model records.

How is this PR tested?

Existing unit/integration tests

Does this PR require documentation update?

No. You can skip the rest of this section.

Release Notes

Is this a user-facing change?

Yes. Give a description of this change to be included in the release notes for MLflow users.

Fix mlflow gc failing with foreign key constraint error when deleting experiments with logged model records.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

area/tracking: Tracking Service, tracking client APIs, autologging

How should the PR be classified in the release notes? Choose one:

rn/bug-fix - A user-facing bug fix worth mentioning in the release notes

Should this PR be included in the next patch release?

Yes (this PR will be cherry-picked and included in the next patch release)
No (this PR will be included in the next minor release)

🤖 Generated with Claude Code

github-actions · 2026-01-21T11:17:56Z

🛠 DevTools 🛠

Install mlflow from this PR

# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/20185/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/20185/merge#subdirectory=libs/skinny

For Databricks, use the following command:

%sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/20185/merge

Copilot

Pull request overview

This pull request adds ON DELETE CASCADE to the experiment_id foreign key constraints in the logged_model_params, logged_model_tags, and logged_model_metrics tables. This change fixes a foreign key constraint error that occurs when mlflow gc attempts to delete experiments that have associated logged model records.

Changes:

Updated SQLAlchemy model definitions to include ondelete="CASCADE" for experiment_id foreign keys
Created database migration to alter existing constraints
Updated schema file to reflect the new constraint definitions

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
mlflow/store/tracking/dbmodels/models.py	Added `ondelete="CASCADE"` parameter to experiment_id foreign key constraints in SqlLoggedModelMetric, SqlLoggedModelParam, and SqlLoggedModelTag classes
mlflow/store/db_migrations/versions/92d5b24e0351_add_cascade_to_logged_model_experiment_fks.py	Created migration to drop and recreate foreign key constraints with CASCADE deletion for the three logged_model tables
tests/resources/db/latest_schema.sql	Updated schema definition to include `ON DELETE CASCADE` in the SQL foreign key constraint definitions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-01-21T11:29:10Z

Documentation preview for 2740ccc is available at:

https://pr-20185--mlflow-docs-preview.netlify.app/docs/latest/

More info

Ignore this comment if this PR does not change the documentation.
The preview is updated when a new commit is pushed to this PR.
This comment was created by this workflow run.
The documentation was built by this workflow run.

harupy · 2026-01-21T11:36:02Z

Verification

I verified the fix by running the new test against MySQL using Docker:

Without the fix (migration removed):

FAILED tests/db/test_tracking_operations.py::test_gc_experiment_with_logged_model_params_and_tags

mlflow.exceptions.MlflowException: (MySQLdb.IntegrityError) (1451, 'Cannot delete or update a parent row: 
a foreign key constraint fails (`mlflowdb`.`logged_model_params`, CONSTRAINT `fk_logged_model_params_experiment_id` 
FOREIGN KEY (`experiment_id`) REFERENCES `experiments` (`experiment_id`))')
[SQL: DELETE FROM experiments WHERE experiments.experiment_id = %s]

With the fix:

PASSED tests/db/test_tracking_operations.py::test_gc_experiment_with_logged_model_params_and_tags

Commands used:

# Build the MySQL service
./tests/db/compose.sh build --build-arg DEPENDENCIES="$(uv run python dev/extract_deps.py)" mlflow-mysql

# Run the test against MySQL
./tests/db/compose.sh run --rm mlflow-mysql pytest tests/db/test_tracking_operations.py::test_gc_experiment_with_logged_model_params_and_tags -v

The test is also added to tests/db/test_tracking_operations.py so it will run as part of the DB integration tests against MySQL, PostgreSQL, and MSSQL.

harupy · 2026-01-21T12:03:36Z

/autoformat

WeichenXu123 · 2026-01-22T08:56:16Z

tests/db/test_tracking_operations.py

+    assert exp_id not in [e.experiment_id for e in experiments]
+
+    with pytest.raises(mlflow.MlflowException, match=r".+ not found"):
+        client.get_logged_model(model.model_id)


the test doesn't seems sufficient,

we need to query the 3 tables SqlLoggedModelTag, SqlLoggedModelParam, SqlLoggedModelMetric to assert related records are deleted.

tests/test_cli.py

PatrickKoss · 2026-02-12T21:52:44Z

Hey @harupy and @WeichenXu123,

Thanks for working on this fix, @harupy! We've independently run into the same issue and arrived at a similar solution, which is how we discovered this PR.

It looks like only the test adjustments are outstanding. We'd be happy to help get this across the finish line if that would be useful.

@harupy, if you have time to address the remaining comments, we'd be glad to let you complete it. However, if you're busy or would prefer us to take it from here, we're also more than willing to take over and push it through. Either way works for us!

@WeichenXu123, from a maintainer perspective, does this approach look good to you? Would you be open to merging once the tests are adjusted?

We think this fix would benefit quite a few users experiencing the same issue, so we're keen to help however we can.

Mytolo · 2026-02-20T07:44:10Z

Hi @harupy, I’ve encountered the same foreign key constraint issue during GC and am very interested in seeing this fix merged.

I noticed there are currently some failing CI checks. I’d be happy to help move this forward—would you like me to look into the test failures or perhaps provide some additional manual testing on my environment to verify the fix?

Thanks for working on this!

Fix for GitHub issue mlflow#20184: GC fails when deleting experiments if there are records in logged_model_params, logged_model_tags, or logged_model_metrics tables that reference the experiment. The experiment_id FK constraints in these tables were missing ON DELETE CASCADE, causing the database to reject experiment deletion. This follows the same pattern used for the logged_models.experiment_id FK which correctly has CASCADE. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Adds a regression test for mlflow#20184 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Adds a test that reproduces the FK constraint error from issue mlflow#20184 when run against MySQL with enforced foreign key constraints. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Replace DB-level ondelete="CASCADE" on experiment_id FKs (which breaks MSSQL due to multiple cascade paths) with an ORM relationship from SqlLoggedModel to SqlExperiment. This lets session.delete(experiment) cascade through ORM: experiment → logged_models → params/tags/metrics. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

tests/resources/db/latest_schema.sql

mlflow/store/tracking/dbmodels/models.py

tests/db/test_tracking_operations.py

tests/test_cli.py

tests/db/test_tracking_operations.py

- Update latest_schema.sql to remove ON DELETE CASCADE from experiment_id FKs on logged_model_params/tags/metrics (matching models.py) - Use CliRunner instead of subprocess in SQLite test - Pass --backend-store-uri explicitly in DB test - Assert child table records (params, tags) are deleted after GC Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Address review feedback to assert all 3 child tables (params, tags, metrics) are cleaned up after GC deletes an experiment with logged models. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

harupy · 2026-02-20T10:27:48Z

Both tests now assert all 3 tables (SqlLoggedModelParam, SqlLoggedModelTag, SqlLoggedModelMetric) are empty after GC. Also added logged_model_metrics creation (via a run + log_metric with model_id) to cover the full cascade path.

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

The DB test (tests/db/test_tracking_operations.py) already covers this against MySQL/PostgreSQL where FK constraints are strictly enforced. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

The ORM cascade fix is DB-engine agnostic, so a SQLite test is sufficient. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

mlflow/store/tracking/dbmodels/models.py

WeichenXu123

#20185 (comment)

WeichenXu123

LGTM!

…flow#20185) Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>

…0185) Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings January 21, 2026 11:17

github-actions bot added area/tracking Tracking service, tracking client APIs, autologging rn/bug-fix Mention under Bug Fixes in Changelogs. labels Jan 21, 2026

harupy changed the title ~~Add CASCADE to logged_model tables experiment_id foreign keys~~ Add CASCADE to logged_model tables experiment_id foreign keys Jan 21, 2026

Copilot started reviewing on behalf of harupy January 21, 2026 11:18 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

harupy mentioned this pull request Jan 21, 2026

[BUG] GC fails if records in logged_model_params table #20184

Closed

1 task

harupy added the team-review Trigger a team review request label Jan 21, 2026

github-actions bot requested review from B-Step62 and WeichenXu123 January 21, 2026 11:37

WeichenXu123 reviewed Jan 22, 2026

View reviewed changes

tests/test_cli.py Show resolved Hide resolved

github-actions bot assigned WeichenXu123 Jan 22, 2026

harupy and others added 3 commits February 20, 2026 18:00

Add test for GC with logged model params and tags

8609c94

Adds a regression test for mlflow#20184 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

harupy force-pushed the fix-gc-logged-model-fk-cascade branch from 52a0cb1 to c61d5d8 Compare February 20, 2026 09:13

harupy requested a review from Copilot February 20, 2026 09:24

Copilot started reviewing on behalf of harupy February 20, 2026 09:25 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

harupy and others added 3 commits February 20, 2026 18:37

Merge branch 'master' into fix-gc-logged-model-fk-cascade

2a3f560

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

harupy and others added 5 commits February 20, 2026 19:33

Move imports to top level in GC tests

fa7dd7d

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Simplify GC test docstring

453a7da

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Simplify child table assertions in GC test

ef6fa36

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Move GC logged model test to test_cli.py with other GC tests

b52d451

The ORM cascade fix is DB-engine agnostic, so a SQLite test is sufficient. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

harupy requested a review from WeichenXu123 February 20, 2026 10:57

Move imports to top level

2740ccc

Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

This comment was marked as outdated.

Sign in to view

WeichenXu123 reviewed Feb 24, 2026

View reviewed changes

mlflow/store/tracking/dbmodels/models.py Show resolved Hide resolved

WeichenXu123 requested changes Feb 24, 2026

View reviewed changes

harupy requested a review from WeichenXu123 February 24, 2026 02:47

WeichenXu123 approved these changes Feb 24, 2026

View reviewed changes

github-actions bot added the v3.10.1 label Feb 24, 2026

harupy added this pull request to the merge queue Feb 24, 2026

Merged via the queue into mlflow:master with commit 08483b8 Feb 24, 2026
61 of 63 checks passed

harupy deleted the fix-gc-logged-model-fk-cascade branch February 24, 2026 03:31

github-actions bot added the size/M label Feb 24, 2026

daniellok-db pushed a commit that referenced this pull request Mar 5, 2026

Add CASCADE to logged_model tables experiment_id foreign keys (#2…

51ef7e5

…0185) Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>

Conversation

harupy commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues/PRs

What changes are proposed in this pull request?

How is this PR tested?

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Should this PR be included in the next patch release?

Uh oh!

github-actions bot commented Jan 21, 2026

Install mlflow from this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harupy commented Jan 21, 2026

Verification

Without the fix (migration removed):

With the fix:

Commands used:

Uh oh!

harupy commented Jan 21, 2026

Uh oh!

WeichenXu123 Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PatrickKoss commented Feb 12, 2026

Uh oh!

Mytolo commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harupy commented Feb 20, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

WeichenXu123 left a comment

Choose a reason for hiding this comment

Uh oh!

WeichenXu123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harupy commented Jan 21, 2026 •

edited

Loading

github-actions bot commented Jan 21, 2026 •

edited

Loading