You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/16 18:44:03 UTC
[PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41126:
URL: https://github.com/apache/superset/pull/41126
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4765194966
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | ab237d16b25619242fd6802ae45744914d750e57 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a38c735cce95700082fd1ed |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-41126--superset-docs-preview.netlify.app](https://deploy-preview-41126--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBEoMgDADAv-RcRixStT_wGZgEtdUmw3CQcfp3ue1lL9Akh2Z4AzxgTuGHa3Xczsb5sWtNDsvCZGT-MGajspdDkq4bmvitg1h3KRPV8wpuwN55RB59b60dnpFaJvjf) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41126--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4722382784
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41126?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 64.30%. Comparing base ([`1f95a6c`](https://app.codecov.io/gh/apache/superset/commit/1f95a6c48652d002a33224f896a20a4c1705c84c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5342d96`](https://app.codecov.io/gh/apache/superset/commit/5342d96b0cf53d857939ba7e56116b0716423152?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41126 +/- ##
==========================================
- Coverage 64.31% 64.30% -0.01%
==========================================
Files 2652 2652
Lines 144799 144799
Branches 33415 33415
==========================================
- Hits 93122 93111 -11
- Misses 50016 50025 +9
- Partials 1661 1663 +2
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.33% <100.00%> (ΓΈ)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.03% <100.00%> (ΓΈ)` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.10% <100.00%> (-0.01%)` | :arrow_down: |
| [presto](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.90% <100.00%> (ΓΈ)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.55% <100.00%> (-0.01%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.72% <100.00%> (ΓΈ)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/41126/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (ΓΈ)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41126?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4813644848
Added the `UPDATING.md` note β manual `DROP CONSTRAINT` (Postgres) / `DROP FOREIGN KEY` (MySQL) with the inspect-first caveat since the names vary. Going with the model fix + doc note, no migration. Thanks for the approve @aminghadersohi.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4766386004
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4765186287
Trimmed the test docstring down to the assertion + a pointer to the model comment β agreed it was redundant.
On M1: the model fix is deliberately the whole scope here. A defensive migration only helps `create_all`-bootstrapped Postgres/MySQL installs, and a `downgrade` can never safely recreate an unsatisfiable constraint, so it'd be a one-way drop introspected by `referred_table`. I'm leaning toward shipping the model fix alone plus an `UPDATING.md` note with the manual `DROP CONSTRAINT` per dialect, rather than a migration that mutates a constraint the canonical chain never created. @aminghadersohi does that split sit right with you, or do you feel strongly the migration belongs in this PR?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4814403214
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41126:
URL: https://github.com/apache/superset/pull/41126#discussion_r3493125215
##########
superset/tags/models.py:
##########
@@ -111,12 +111,14 @@ class TaggedObject(Model, AuditMixinNullable):
__tablename__ = "tagged_object"
id = Column(Integer, primary_key=True)
tag_id = Column(Integer, ForeignKey("tag.id"))
- object_id = Column(
- Integer,
- ForeignKey("dashboards.id"),
- ForeignKey("slices.id"),
- ForeignKey("saved_query.id"),
- )
+ # ``object_id`` is a polymorphic reference disambiguated by ``object_type``;
+ # the same value can point at a dashboard, chart or saved query. It must not
+ # carry a foreign key to any single table: declaring FKs to dashboards,
+ # slices and saved_query at once is unsatisfiable (a row would have to exist
+ # in all three) and breaks tagging, e.g. tagging a dashboard fails the
+ # slices FK. The original migration (c82ee8a39623) defined no FK here. See
+ # issue #35941.
+ object_id = Column(Integer)
Review Comment:
**Suggestion:** Add an explicit type annotation for this model attribute so the modified variable declaration is type-hinted. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly modified Python model attribute that can be annotated, but it is declared without any type hint. That matches the custom rule requiring type hints for relevant variables in modified code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=88c14632697b41f4bcdab242376d312b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=88c14632697b41f4bcdab242376d312b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tags/models.py
**Line:** 121:121
**Comment:**
*Custom Rule: Add an explicit type annotation for this model attribute so the modified variable declaration is type-hinted.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=bf4df0cb16add078f218cf5414ff716f1b7af93477b2aa773db7711c44ba8b1a&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=bf4df0cb16add078f218cf5414ff716f1b7af93477b2aa773db7711c44ba8b1a&reaction=dislike'>π</a>
##########
tests/unit_tests/tags/models_test.py:
##########
@@ -261,3 +261,16 @@ def test_tag_name_type_after_database_operation() -> None:
assert added_tag.name.__class__ is str, (
"Tag name should be exactly str type, not a subclass"
)
+
+
+def test_tagged_object_object_id_has_no_foreign_keys() -> None:
+ """Guard against ``tagged_object.object_id`` regaining a foreign key.
+
+ It is a polymorphic reference (see the rationale on the model); declaring
+ FKs there is unsatisfiable and breaks tagging (issue #35941).
+ """
+ foreign_keys = TaggedObject.__table__.c.object_id.foreign_keys
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced local variable to satisfy the required Python type-hint rule. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The new local variable is introduced without a type hint even though its type is concrete and readily inferable from the SQLAlchemy attribute. This matches the Python type-hint rule for newly added variables that can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a8b62c8fbdea4f9797fadd7e2ed63144&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a8b62c8fbdea4f9797fadd7e2ed63144&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/tags/models_test.py
**Line:** 272:272
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced local variable to satisfy the required Python type-hint rule.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=f8f60d0567de804237c88af000726a97ac878b5ad50e92d67e635fed0f28fe61&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=f8f60d0567de804237c88af000726a97ac878b5ad50e92d67e635fed0f28fe61&reaction=dislike'>π</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4815720014
@villebro might want to sanity check this one (or pass the buck to someone else qualified) for human oversight before I YOLO it...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4836105098
**Bito Automatic Review Skipped β PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4722167085
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #41126:
URL: https://github.com/apache/superset/pull/41126#discussion_r3450206418
##########
superset/tags/models.py:
##########
@@ -111,12 +111,14 @@ class TaggedObject(Model, AuditMixinNullable):
__tablename__ = "tagged_object"
id = Column(Integer, primary_key=True)
tag_id = Column(Integer, ForeignKey("tag.id"))
- object_id = Column(
- Integer,
- ForeignKey("dashboards.id"),
- ForeignKey("slices.id"),
- ForeignKey("saved_query.id"),
- )
+ # ``object_id`` is a polymorphic reference disambiguated by ``object_type``;
+ # the same value can point at a dashboard, chart or saved query. It must not
+ # carry a foreign key to any single table: declaring FKs to dashboards,
+ # slices and saved_query at once is unsatisfiable (a row would have to exist
+ # in all three) and breaks tagging, e.g. tagging a dashboard fails the
+ # slices FK. The original migration (c82ee8a39623) defined no FK here. See
+ # issue #35941.
+ object_id = Column(Integer)
Review Comment:
This is a deliberate scope call, covered in the PR body β the canonical migration chain (`c82ee8a39623` onward) never created these FKs, so only `create_all`-bootstrapped schemas carry them. A `downgrade` could never safely recreate an unsatisfiable constraint, so any migration would be a one-way introspected drop. I've got that exact tradeoff open with @aminghadersohi to decide whether it belongs here or in `UPDATING.md`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41126:
URL: https://github.com/apache/superset/pull/41126#discussion_r3449988141
##########
superset/tags/models.py:
##########
@@ -111,12 +111,14 @@ class TaggedObject(Model, AuditMixinNullable):
__tablename__ = "tagged_object"
id = Column(Integer, primary_key=True)
tag_id = Column(Integer, ForeignKey("tag.id"))
- object_id = Column(
- Integer,
- ForeignKey("dashboards.id"),
- ForeignKey("slices.id"),
- ForeignKey("saved_query.id"),
- )
+ # ``object_id`` is a polymorphic reference disambiguated by ``object_type``;
+ # the same value can point at a dashboard, chart or saved query. It must not
+ # carry a foreign key to any single table: declaring FKs to dashboards,
+ # slices and saved_query at once is unsatisfiable (a row would have to exist
+ # in all three) and breaks tagging, e.g. tagging a dashboard fails the
+ # slices FK. The original migration (c82ee8a39623) defined no FK here. See
+ # issue #35941.
+ object_id = Column(Integer)
Review Comment:
**Suggestion:** Updating the ORM model alone does not remediate databases that were already created with the old unsatisfiable foreign keys on `tagged_object.object_id`; those constraints will remain in the physical schema and tagging will keep failing at runtime. Add an Alembic migration that explicitly drops any existing FK constraints on `tagged_object.object_id` (with dialect-safe checks) so existing environments are actually repaired. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Tag REST API fails creating tags on legacy schemas.
- β Automatic tag relationships fail, blocking TAGGING_SYSTEM usage.
- β οΈ Developers must manually drop broken FKs on databases.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Start from a Superset deployment whose database schema was originally created from the
pre-PR ORM models using SQLAlchemy `create_all`, so the `tagged_object` table has
`object_id` defined with multiple foreign keys to dashboards, slices, and saved_query (as
described in the PR summary). After applying the current PR, note that the ORM model now
defines `object_id = Column(Integer)` with no foreign key in
`superset/tags/models.py:19-40`, but existing database constraints are unchanged because
migrations do not touch them.
2. Confirm that no Alembic migration drops foreign keys on `tagged_object.object_id`: the
initial table is created in
`superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py:73-89`
with `object_id = Column(Integer)` and no FK; later migrations
`def97f26fdfb_add_index_to_tagged_object.py:32-35`,
`07f9a902af1b_drop_postgres_enum_constrains_for_tags.py:33-41`, and
`96164e3017c6_tagged_object_unique_constraint.py:37-50` only add an index, change enum
types, and add a unique constraint, respectively, and never drop any FK constraints that
may already exist on `tagged_object.object_id` in a schema created via `create_all`.
3. Upgrade the Superset application code to this PR version without manually altering the
database schema, enable the `TAGGING_SYSTEM` feature flag, and create a custom tag for an
existing dashboard via the Tag REST API endpoint `POST api/v1/tag/`. This request is
handled by `TagRestApi.post` in `superset/tags/api.py:152-199`, which invokes
`CreateCustomTagWithRelationshipsCommand.run` (`superset/commands/tag/create.py:102-121`),
and that in turn calls `TagDAO.create_tag_relationship` (`superset/daos/tag.py:71-129`) to
create `TaggedObject` rows.
4. In `TagDAO.create_tag_relationship`, observe that Superset instantiates
`TaggedObject(object_id=object_id, object_type=object_type, tag=tag)` at
`superset/daos/tag.py:106-111` and adds it to the session; when the session flushes, the
INSERT into `tagged_object` still hits the pre-existing database-level foreign keys on
`object_id`. Because the dashboard id does not simultaneously exist in `slices` and
`saved_query`, the database raises a `ForeignKeyViolation` and tag creation fails,
demonstrating that removing the FK from the ORM at `superset/tags/models.py:32` without an
Alembic migration leaves previously created schemas broken.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1060af8e17c043b3bc3196ecb87d913c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1060af8e17c043b3bc3196ecb87d913c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tags/models.py
**Line:** 121:121
**Comment:**
*Incomplete Implementation: Updating the ORM model alone does not remediate databases that were already created with the old unsatisfiable foreign keys on `tagged_object.object_id`; those constraints will remain in the physical schema and tagging will keep failing at runtime. Add an Alembic migration that explicitly drops any existing FK constraints on `tagged_object.object_id` (with dialect-safe checks) so existing environments are actually repaired.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=504d4bd66fe5bab854fce2947b0c5e49f36d79456db1fce339a6bbe1ae3cffaf&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41126&comment_hash=504d4bd66fe5bab854fce2947b0c5e49f36d79456db1fce339a6bbe1ae3cffaf&reaction=dislike'>π</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tags): remove unsatisfiable foreign keys from tagged_object.object_id [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41126:
URL: https://github.com/apache/superset/pull/41126#issuecomment-4765207966
<!-- Bito Reply -->
The flagged issue is correct. Updating the ORM model prevents new foreign key constraints from being created, but it does not remove existing constraints from databases already initialized with the previous model definition. To resolve this, an Alembic migration is required to drop these constraints.
To implement this, create a new migration file (e.g., `superset/migrations/versions/2026-06-22_drop_tagged_object_fk.py`) that uses `op.drop_constraint` with dialect-specific checks to remove the foreign keys on `tagged_object.object_id`.
There are no other comments in the provided review context to address.
**superset/migrations/versions/2026-06-22_drop_tagged_object_fk.py**
```
def upgrade():
bind = op.get_bind()
inspector = sa.inspect(bind)
constraints = [c['name'] for c in inspector.get_foreign_keys('tagged_object') if 'object_id' in c['constrained_columns']]
for constraint in constraints:
op.drop_constraint(constraint, 'tagged_object', type_='foreignkey')
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org