rusackas opened a new pull request, #41126:
URL: https://github.com/apache/superset/pull/41126
### SUMMARY
`tagged_object.object_id` is a polymorphic reference (it points at a
dashboard, chart, or saved query, disambiguated by `object_type`). The model in
`superset/tags/models.py`, however, declared **three foreign keys on the single
column at once**:
```python
object_id = Column(
Integer,
ForeignKey("dashboards.id"),
ForeignKey("slices.id"),
ForeignKey("saved_query.id"),
)
```
Those constraints are conjunctive, so a row can only be inserted if the same
`object_id` value exists in `dashboards` **and** `slices` **and** `saved_query`
simultaneously, which is impossible. On any schema that actually enforces them,
every tag insert fails. With `TAGGING_SYSTEM` enabled, the `after_insert` hook
tags a newly created dashboard and the insert blows up with a
`ForeignKeyViolation` against the `slices` table (issue #35941):
```
insert or update on table "tagged_object" violates foreign key constraint
"tagged_object_object_id_fkey"
DETAIL: Key (object_id)=(72) is not present in table "slices".
```
The original migration that created this table (`c82ee8a39623`, 2018)
defined `object_id = Column(Integer)` with **no** foreign key. No migration has
ever added one, so a schema built via `superset db upgrade` doesn't carry the
constraint and the bug is latent there, but a schema built from the models
(`create_all`, as some dev/Docker setups do) materializes all three FKs and
breaks. This restores the model to match the migration's polymorphic intent.
The ORM tag relationships on `Dashboard`, `Slice` and `SavedQuery` use
`secondary="tagged_object"` with explicit `primaryjoin`/`secondaryjoin`
conditions; SQLAlchemy resolves the join direction from those conditions and
the real `tag_id` FK, so they are unaffected by removing the `object_id` FKs
(verified: mappers configure cleanly and emit identical JOIN SQL with no
warnings).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - model/schema change only.
**Before:** creating/tagging a dashboard with `TAGGING_SYSTEM: True` raises
`IntegrityError: ForeignKeyViolation ... not present in table "slices"`.
**After:** `tagged_object.object_id` carries no foreign key; tagging works
for dashboards, charts and saved queries.
### TESTING INSTRUCTIONS
Automated (deterministic, no DB FK enforcement needed):
```bash
pytest
tests/unit_tests/tags/models_test.py::test_tagged_object_object_id_has_no_foreign_keys
```
The new test asserts `TaggedObject.__table__.c.object_id.foreign_keys ==
set()`. It is red on the old three-FK model and green after this change,
guarding against the model drifting back.
Manual:
1. Set `"TAGGING_SYSTEM": True` in `superset_config.py`.
2. Build the metadata DB from the models (`create_all`) on a backend that
enforces FKs (e.g. Postgres).
3. Create a new dashboard via the UI. Before this change it fails with the
FK violation above; after, it succeeds and a `tagged_object` row is written.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #35941
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
**Note on migrations:** since no migration ever created these FK
constraints, the canonical migration-built schema needs no change. Deployments
whose schema was built from the models (`create_all`) and that already enforce
the broken constraints would need them dropped to recover. I deliberately
scoped this PR to the model fix rather than guessing at a fragile cross-dialect
"drop constraint if exists" migration (the auto-generated names differ per
backend, e.g. `..._fkey`, `..._fkey1`, `..._fkey2`). Happy to add a defensive
migration if maintainers would prefer one.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
