fix: prevent ForeignKeyViolation when deleting flows with traces#12499
fix: prevent ForeignKeyViolation when deleting flows with traces#12499octo-patch wants to merge 1 commit intolangflow-ai:mainfrom
Conversation
Fixes langflow-ai#12346 When deleting a flow that has associated execution traces, PostgreSQL raises a ForeignKeyViolation because the span.trace_id FK was created without ON DELETE CASCADE, leaving spans dangling when traces are deleted during the flow->trace cascade chain. Two-pronged fix: - cascade_delete_flow() now explicitly deletes SpanTable rows (grouped by trace) and TraceTable rows before deleting the Flow, matching the existing pattern used for Message/Transaction/VertexBuild tables. - SpanTable.trace_id Field gains ondelete="CASCADE" so new deployments get the correct schema from the ORM. - New Alembic migration (a3f8b2c91d04) drops and re-creates the span.trace_id FK with ON DELETE CASCADE for existing deployments.
WalkthroughThis PR implements a two-pronged fix for flow deletion failures caused by missing cascade constraints on the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings, 2 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/backend/base/langflow/alembic/versions/a3f8b2c91d04_add_ondelete_cascade_to_span_trace_id_fk.py`:
- Around line 23-29: The downgrade() currently returns early when
_get_fk_constraint_name(...) yields None (so SQLite leaves the CASCADE FK in
place); update downgrade() to handle fk_name is None by falling back to
inspecting inspector.get_foreign_keys(table_name) to find the FK entry by
constrained_columns and referred_table/columns and then recreate the FK without
ON DELETE CASCADE (i.e., drop and re-add the FK constraint or recreate the table
schema with the non-CASCADE FK) instead of returning; modify
_get_fk_constraint_name/downgrade to use the FK metadata (constrained_columns,
referred_table, referred_columns) to identify and rebuild the constraint when
the inspector cannot provide a name.
In `@src/backend/base/langflow/api/utils/core.py`:
- Around line 356-358: The current code materializes trace IDs by calling
session.exec(sm_select(TraceTable.id)...) and then uses Python-level trace_ids
in SpanTable.trace_id.in_(trace_ids), which will build a huge IN(...) list for
large flows; change to perform a SQL-level subquery delete so the DB deletes
spans directly without loading IDs into Python—replace the two-step pattern with
a single delete against SpanTable where SpanTable.trace_id is in a subquery
selecting TraceTable.id (use the existing
sm_select/TraceTable/SpanTable/delete/session.exec symbols to construct the
subquery so the database executes the deletion).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71e4f149-bfbe-4305-a2f9-f7d7ae661c19
📒 Files selected for processing (3)
src/backend/base/langflow/alembic/versions/a3f8b2c91d04_add_ondelete_cascade_to_span_trace_id_fk.pysrc/backend/base/langflow/api/utils/core.pysrc/backend/base/langflow/services/database/models/traces/model.py
| def _get_fk_constraint_name(conn, table_name: str, column_name: str) -> str | None: | ||
| """Find the foreign key constraint name for a given column.""" | ||
| inspector = sa.inspect(conn) | ||
| for fk in inspector.get_foreign_keys(table_name): | ||
| if column_name in fk["constrained_columns"]: | ||
| return fk["name"] | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether SQLite exposes FK names via SQLAlchemy inspector in your env.
python - <<'PY'
import sqlalchemy as sa
engine = sa.create_engine("sqlite:///:memory:")
with engine.begin() as conn:
conn.exec_driver_sql("PRAGMA foreign_keys=ON")
conn.exec_driver_sql("CREATE TABLE trace (id TEXT PRIMARY KEY)")
conn.exec_driver_sql(
"CREATE TABLE span (id TEXT PRIMARY KEY, trace_id TEXT, "
"FOREIGN KEY(trace_id) REFERENCES trace(id) ON DELETE CASCADE)"
)
insp = sa.inspect(engine)
fks = insp.get_foreign_keys("span")
print("FK metadata:", fks)
print("FK names:", [fk.get("name") for fk in fks])
PYRepository: langflow-ai/langflow
Length of output: 186
🏁 Script executed:
cd /tmp && find . -path "*/langflow/alembic/versions/a3f8b2c91d04*" -type f 2>/dev/null | head -5Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
git ls-files | grep -E "(alembic|migration)" | head -20Repository: langflow-ai/langflow
Length of output: 1430
🏁 Script executed:
# Look for the migration file mentioned in the review
git ls-files | grep "a3f8b2c91d04"Repository: langflow-ai/langflow
Length of output: 163
🏁 Script executed:
# Check pyproject.toml or requirements.txt for dependencies
git ls-files | grep -E "(requirements|setup|pyproject|poetry)" | head -10Repository: langflow-ai/langflow
Length of output: 433
🏁 Script executed:
# Find and examine the migration file
find . -name "*a3f8b2c91d04*" -type f 2>/dev/nullRepository: langflow-ai/langflow
Length of output: 165
🏁 Script executed:
cat -n src/backend/base/langflow/alembic/versions/a3f8b2c91d04_add_ondelete_cascade_to_span_trace_id_fk.pyRepository: langflow-ai/langflow
Length of output: 2605
🏁 Script executed:
# Check the pyproject.toml for SQLAlchemy version
cat pyproject.toml | grep -A 5 -B 5 "sqlalchemy"Repository: langflow-ai/langflow
Length of output: 406
🏁 Script executed:
# Look for similar migration patterns in the codebase
rg "get_foreign_keys" --type pyRepository: langflow-ai/langflow
Length of output: 3143
🏁 Script executed:
# Check the migration utility module to understand how FK names are handled
cat src/backend/base/langflow/utils/migration.py | head -100Repository: langflow-ai/langflow
Length of output: 2174
🏁 Script executed:
# Look at the previous similar migration to understand the pattern
cat -n src/backend/base/langflow/alembic/versions/0e6138e7a0c2_add_ondelete_cascade_to_file_user_id_fk.pyRepository: langflow-ai/langflow
Length of output: 2580
🏁 Script executed:
# Check test files for migration validation and FK behavior
cat -n src/backend/tests/unit/alembic/test_migration_execution.py | head -150Repository: langflow-ai/langflow
Length of output: 5611
🏁 Script executed:
# Look for any database configuration or SQLite-specific handling in migrations
rg "sqlite" --type py | head -20Repository: langflow-ai/langflow
Length of output: 2249
🏁 Script executed:
# Check if there are any tests specifically for the downgrade behavior
find . -name "*test*migration*" -type f -exec grep -l "downgrade" {} \;Repository: langflow-ai/langflow
Length of output: 247
🏁 Script executed:
# Look at how other migrations handle the FK name None case
rg "if fk_name is None" --type py -A 3Repository: langflow-ai/langflow
Length of output: 3318
🏁 Script executed:
# Check the SQLAlchemy version being used
cat pyproject.toml | grep -A 2 "sqlalchemy"Repository: langflow-ai/langflow
Length of output: 179
🏁 Script executed:
# Check test files for migration behavior testing
cat -n src/backend/tests/unit/alembic/test_existing_migrations.pyRepository: langflow-ai/langflow
Length of output: 4816
🏁 Script executed:
# Check test guidelines for migrations
cat -n src/backend/tests/unit/alembic/test_migration_guidelines.py | head -150Repository: langflow-ai/langflow
Length of output: 5671
🏁 Script executed:
# Look at another similar migration to see the complete pattern
cat -n src/backend/base/langflow/alembic/versions/59a272d6669a_ensure_trace_flow_id_cascade.pyRepository: langflow-ai/langflow
Length of output: 3489
🏁 Script executed:
# Verify if there are tests for downgrade reversibility
rg "downgrade" --type py src/backend/tests/unit/alembic/ -A 2 -B 2Repository: langflow-ai/langflow
Length of output: 5985
🏁 Script executed:
# Check if any of these migrations have been tested for rollback
rg "test.*downgrade\|def test.*rollback" --type pyRepository: langflow-ai/langflow
Length of output: 46
Downgrade cannot properly reverse when FK names are unavailable on SQLite.
The downgrade() method returns early when fk_name is None (lines 61-62), leaving the FK with CASCADE semantics instead of restoring the non-CASCADE version. SQLite does not reliably expose FK constraint names via SQLAlchemy's inspector API, making this downgrade path incomplete.
Handle the None case explicitly: either track the constraint differently, store metadata in comments/triggers, or update downgrade to recreate the FK by dropping and recreating it when name lookup fails.
Also applies to: 61-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/backend/base/langflow/alembic/versions/a3f8b2c91d04_add_ondelete_cascade_to_span_trace_id_fk.py`
around lines 23 - 29, The downgrade() currently returns early when
_get_fk_constraint_name(...) yields None (so SQLite leaves the CASCADE FK in
place); update downgrade() to handle fk_name is None by falling back to
inspecting inspector.get_foreign_keys(table_name) to find the FK entry by
constrained_columns and referred_table/columns and then recreate the FK without
ON DELETE CASCADE (i.e., drop and re-add the FK constraint or recreate the table
schema with the non-CASCADE FK) instead of returning; modify
_get_fk_constraint_name/downgrade to use the FK metadata (constrained_columns,
referred_table, referred_columns) to identify and rebuild the constraint when
the inspector cannot provide a name.
| trace_ids = (await session.exec(sm_select(TraceTable.id).where(TraceTable.flow_id == flow_id))).all() | ||
| if trace_ids: | ||
| await session.exec(delete(SpanTable).where(SpanTable.trace_id.in_(trace_ids))) |
There was a problem hiding this comment.
Avoid materializing all trace IDs before deleting spans.
Loading all IDs into Python and then building a large IN (...) list can degrade badly for large flows. Use a subquery delete directly.
💡 Proposed change
- trace_ids = (await session.exec(sm_select(TraceTable.id).where(TraceTable.flow_id == flow_id))).all()
- if trace_ids:
- await session.exec(delete(SpanTable).where(SpanTable.trace_id.in_(trace_ids)))
+ trace_ids_subquery = sm_select(TraceTable.id).where(TraceTable.flow_id == flow_id)
+ await session.exec(delete(SpanTable).where(SpanTable.trace_id.in_(trace_ids_subquery)))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/base/langflow/api/utils/core.py` around lines 356 - 358, The
current code materializes trace IDs by calling
session.exec(sm_select(TraceTable.id)...) and then uses Python-level trace_ids
in SpanTable.trace_id.in_(trace_ids), which will build a huge IN(...) list for
large flows; change to perform a SQL-level subquery delete so the DB deletes
spans directly without loading IDs into Python—replace the two-step pattern with
a single delete against SpanTable where SpanTable.trace_id is in a subquery
selecting TraceTable.id (use the existing
sm_select/TraceTable/SpanTable/delete/session.exec symbols to construct the
subquery so the database executes the deletion).
|
I just was wondering that when this PR would be merged. This is quite a bug which has to be fixed early. |
2 participants
Fixes #12346
Problem
Deleting a flow that has associated execution traces raises a
ForeignKeyViolationin PostgreSQL. The root cause is that thespan.trace_idFK was created withoutON DELETE CASCADEin migration3478f0bd6ccb. When PostgreSQL cascades theflow → tracedeletion, it then tries to deletetracerows, butspanrecords still reference them — causing a constraint error.There are two compounding issues:
span.trace_idFK missingON DELETE CASCADEat the database levelcascade_delete_flow()never explicitly deletesTraceTableorSpanTablerecords, relying entirely on the broken DB-level cascade chainSolution
Application-level (defence-in-depth):
cascade_delete_flow()now explicitly deletesSpanTablerows (keyed by trace IDs for the flow) andTraceTablerows before deleting theFlow, matching the existing pattern forMessageTable,TransactionTable,VertexBuildTable, andFlowVersion.Database-level: New Alembic migration
a3f8b2c91d04drops and re-creates thespan.trace_idFK withON DELETE CASCADE. Usesbatch_alter_tablefor SQLite compatibility and introspects the actual FK constraint name (handles both auto-generated and named constraints).ORM model:
SpanTable.trace_idField gainsondelete="CASCADE"so new deployments get the correct schema from the start.Testing
ForeignKeyViolationalembic upgrade headon both PostgreSQL and SQLitealembic downgrade -1to verify reversibilitySummary by CodeRabbit