You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2025/10/06 23:26:00 UTC

[PR] migration: update tag unique constraint to (name, type) [superset]

eschutho opened a new pull request, #35542:
URL: https://github.com/apache/superset/pull/35542

   ### SUMMARY
   
   This PR adds a database migration to update the tag table unique constraint from name-only to a composite constraint on `(name, type)`.
   
   This migration supports the fix in #35541 that allows the same tag name to exist with different tag types. For example, "type:dashboard" can be both a system tag (type='type') and a custom tag (type='custom').
   
   **Migration changes:**
   - Drops the old unique constraint `tag_name_key` on the `name` column
   - Creates new composite unique constraint `uix_tag_name_type` on `(name, type)`
   
   **Why this is needed:**
   The current single-column unique constraint on `name` prevents the same tag name from being used with different types. This causes issues when:
   1. A system tag like "type:dashboard" (type='type') exists
   2. A user tries to create a custom tag with the same name (type='custom')
   3. The database rejects it due to the name-only unique constraint
   
   The composite constraint allows multiple tags with the same name as long as they have different types.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A - Database migration
   
   ### TESTING INSTRUCTIONS
   1. Apply the migration: `superset db upgrade`
   2. Verify the constraint was updated:
      - PostgreSQL: `\d tag` should show `uix_tag_name_type` constraint
      - MySQL: `SHOW CREATE TABLE tag` should show the composite unique key
   3. Test that tags with same name but different types can coexist:
      ```python
      # In Superset shell
      from superset.tags.models import Tag, TagType
      from superset import db
      
      # Create system tag
      tag1 = Tag(name="foo", type=TagType.type)
      db.session.add(tag1)
      db.session.commit()
      
      # Create custom tag with same name - should succeed
      tag2 = Tag(name="foo", type=TagType.custom)
      db.session.add(tag2)
      db.session.commit()  # Should not raise IntegrityError
      ```
   4. Test downgrade: `superset db downgrade -1`
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue: Related to #35541
   - [ ] Required feature flags: None
   - [ ] Changes UI: No
   - [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [x] Migration is atomic, supports rollback & is backwards-compatible
     - [x] Confirm DB migration upgrade and downgrade tested
     - [x] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API: No
   - [ ] Removes existing feature or API: No
   
   **Migration Details:**
   - **Atomic**: Yes - single DDL operation wrapped in try/except
   - **Rollback support**: Yes - downgrade() reverts to original constraint
   - **Backwards compatible**: Yes - existing data remains valid
   - **Runtime**: < 1 second on tables with millions of rows (constraint operations are fast)
   - **Downtime**: None - constraint changes don't lock the table for writes on modern databases
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   
   Co-Authored-By: Claude <no...@anthropic.com>


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-4699834431

   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: update tag unique constraint to (name, type) [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5685353684

   @eschutho since my nudge in June, @sadpandajoe flagged two real gaps here: the dedup delete around line 93 can drop a live tag when `name` is NULL, and there's no regression test for the new (name, type) uniqueness. On top of that CI's red across mysql/postgres/sqlite/cypress/playwright, and the migration hardcodes the old constraint as `tag_name_key`, which isn't guaranteed on non-Postgres backends. Worth a pass before this can land. Holler if you want me/us to push some commits.


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-4699387509

   @eschutho just a nudge - this still needs the Revises/down_revision bumped to the current migration head per @michael-s-molina, and mistercrunch's nudge toward the migration_utils helpers seems worth folding in while you're at it. I'll update the branch in the meantime for good measure. 


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6307965903

### ⚠️ Possible DB migration conflict

@eschutho the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.

To check and, if needed, fix this:

1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: update tag unique constraint to (name, type) [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3825416433


##########
superset/tags/models.py:
##########
@@ -104,6 +104,8 @@ class Tag(CoreTag, AuditMixinNullable):
         security_manager.user_model, secondary=user_favorite_tag_table
     )
 
+    __table_args__ = (UniqueConstraint("name", "type", name="uix_tag_name_type"),)

Review Comment:
   This changes the persisted uniqueness contract, but there is no regression coverage for it. Could this add a database integration test that creates two tags with the same name and different `TagType` values, then asserts both persist (while the same name/type still raises)?



-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3960030748


##########
superset/tags/models.py:
##########
@@ -104,6 +104,8 @@ class Tag(CoreTag, AuditMixinNullable):
         security_manager.user_model, secondary=user_favorite_tag_table
     )
 
+    __table_args__ = (UniqueConstraint("name", "type", name="uix_tag_name_type"),)

Review Comment:
   This has a destructive path in the current code: `DeleteTagsCommand.validate()` accepts a name when `find_by_name()` returns any matching row, while `delete_tags()` then deletes every row with that name. Once a custom tag shares a system-tag name, an otherwise valid delete request can remove both variants and their associations. Could these operations take a tag ID or `(name, type)` so validation and deletion select the same row?



-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5941599370

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [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 #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r4291159173


##########
superset/tags/models.py:
##########
@@ -108,6 +108,8 @@ class Tag(CoreTag, AuditMixinNullable):
         security_manager.user_model, secondary=user_favorite_tag_table
     )
 
+    __table_args__ = (UniqueConstraint("name", "type", name="uix_tag_name_type"),)

Review Comment:
   **Suggestion:** With same-name custom and system rows, name-only validation can pick the owned custom tag, then bulk deletion removes both, bypassing the system-tag deletion guard.
   
   **Assessment:** 🔴 `Critical` · 🔁 `Occurrence: Sometimes` · 🏷️ `Security`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=23a162cd3a7149fbb00efdab9bc4d28d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=23a162cd3a7149fbb00efdab9bc4d28d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <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:** 111:111
   **Comment:**
   	*Security: With same-name custom and system rows, name-only validation can pick the owned custom tag, then bulk deletion removes both, bypassing the system-tag deletion guard.
   
   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%2F35542&comment_hash=e64305fc283d136fe585d46d4ccca338c8c6420515ba48bb3f1bc0d86a11cdac&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=e64305fc283d136fe585d46d4ccca338c8c6420515ba48bb3f1bc0d86a11cdac&reaction=dislike'>👎</a>



##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c179af013f48
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c179af013f48"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"

Review Comment:
   **Suggestion:** The original constraint is unnamed, so MySQL creates its unique index as `name`, not `tag_name_key`; dropping this hard-coded name fails and stops the upgrade.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0d966649f50e41f09673368923d377b1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0d966649f50e41f09673368923d377b1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 56:56
   **Comment:**
   	*Api Mismatch: The original constraint is unnamed, so MySQL creates its unique index as `name`, not `tag_name_key`; dropping this hard-coded name fails and stops the upgrade.
   
   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%2F35542&comment_hash=d32c87758855e305602b3fc98bdca53996b8aa84a850b4e414f88000b0b9e869&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=d32c87758855e305602b3fc98bdca53996b8aa84a850b4e414f88000b0b9e869&reaction=dislike'>👎</a>



##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c179af013f48
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c179af013f48"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    bind = op.get_bind()
+
+    # Reflect the current database state to get existing tables
+    metadata.reflect(bind=bind)
+
+    # Delete duplicate tags if any, keeping the one with the lowest ID
+    min_id_subquery = (
+        select(
+            [
+                func.min(tag_table.c.id).label("min_id"),
+                tag_table.c.name,
+                tag_table.c.type,
+            ]
+        )
+        .group_by(
+            tag_table.c.name,
+            tag_table.c.type,
+        )
+        .alias("min_ids")
+    )
+
+    delete_query = tag_table.delete().where(
+        tag_table.c.id.notin_(select([min_id_subquery.c.min_id]))
+    )
+
+    bind.execute(delete_query)

Review Comment:
   **Suggestion:** The cleanup can delete duplicate NULL-name tags that still have `tagged_object` references; the foreign key rejects this DELETE, aborting migration on databases enforcing it.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Incomplete implementation`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=67b296ed7a3b4201a4fc91baf0252345&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=67b296ed7a3b4201a4fc91baf0252345&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 89:93
   **Comment:**
   	*Incomplete Implementation: The cleanup can delete duplicate NULL-name tags that still have `tagged_object` references; the foreign key rejects this DELETE, aborting migration on databases enforcing it.
   
   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%2F35542&comment_hash=7935d734b39bda049daf83c3e69002aaf221785a67d3429d0239624f522c2e94&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=7935d734b39bda049daf83c3e69002aaf221785a67d3429d0239624f522c2e94&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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6461080844

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [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 #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3408401991


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    bind = op.get_bind()
+
+    # Reflect the current database state to get existing tables
+    metadata.reflect(bind=bind)
+
+    # Delete duplicate tags if any, keeping the one with the lowest ID
+    min_id_subquery = (
+        select(
+            [
+                func.min(tag_table.c.id).label("min_id"),
+                tag_table.c.name,
+                tag_table.c.type,
+            ]
+        )
+        .group_by(
+            tag_table.c.name,
+            tag_table.c.type,
+        )
+        .alias("min_ids")
+    )
+
+    delete_query = tag_table.delete().where(
+        tag_table.c.id.notin_(select([min_id_subquery.c.min_id]))
+    )
+
+    bind.execute(delete_query)
+
+    # Drop the old unique constraint on name only
+    utils.drop_unique_constraint(op, old_constraint_name, table_name)
+

Review Comment:
   **🔴 Architect Review — CRITICAL**
   
   The migration hardcodes the old unique constraint name as "tag_name_key", but the original tag table was created in migration c82ee8a39623 via Column(..., unique=True) on a bare declarative_base() (no naming convention), so the constraint name is dialect-dependent; on non-Postgres backends the unique index will not be called "tag_name_key", and utils.drop_unique_constraint(op, "tag_name_key", "tag") will emit a DROP CONSTRAINT for a non-existent constraint and fail.
   
   **Suggestion:** Before dropping, inspect existing unique constraints on the "tag" table and drop the one whose column set is ["name"] (using Inspector or by extending migration_utils) instead of assuming the name "tag_name_key", so the migration works across all supported dialects (Postgres, MySQL, etc.).
   
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7c440293eaf04f67a817c53f6d37c960&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7c440293eaf04f67a817c53f6d37c960&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 an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
   
   **Path:** superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 56:97
   **Comment:**
   	*CRITICAL: The migration hardcodes the old unique constraint name as "tag_name_key", but the original tag table was created in migration c82ee8a39623 via Column(..., unique=True) on a bare declarative_base() (no naming convention), so the constraint name is dialect-dependent; on non-Postgres backends the unique index will not be called "tag_name_key", and utils.drop_unique_constraint(op, "tag_name_key", "tag") will emit a DROP CONSTRAINT for a non-existent constraint and fail.
   
   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.
   If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
   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>



##########
superset/tags/models.py:
##########
@@ -104,6 +104,8 @@ class Tag(CoreTag, AuditMixinNullable):
         security_manager.user_model, secondary=user_favorite_tag_table
     )
 
+    __table_args__ = (UniqueConstraint("name", "type", name="uix_tag_name_type"),)

Review Comment:
   **🟠 Architect Review — HIGH**
   
   After changing the unique constraint to (name, type), TagDAO and tag commands still resolve tags by name only (e.g., TagDAO.find_by_name/find_by_names and DeleteTagsCommand/DeleteTaggedObjectCommand), so flows that previously relied on name being globally unique can now target the wrong tag row when the same name exists across different TagType values.
   
   **Suggestion:** Make all name-based tag operations type-aware (or use tag IDs) before relying on (name, type) uniqueness—for example, add a type filter to TagDAO.find_by_name/find_by_names and update DeleteTagsCommand/DeleteTaggedObjectCommand and TagDAO.delete_tags/delete_tagged_object to work with (name, type) or tag IDs instead of bare names.
   
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6d162ccc3936416c9ccb2c4d9a509d92&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6d162ccc3936416c9ccb2c4d9a509d92&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 an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
   
   **Path:** superset/tags/models.py
   **Line:** 95:107
   **Comment:**
   	*HIGH: After changing the unique constraint to (name, type), TagDAO and tag commands still resolve tags by name only (e.g., TagDAO.find_by_name/find_by_names and DeleteTagsCommand/DeleteTaggedObjectCommand), so flows that previously relied on name being globally unique can now target the wrong tag row when the same name exists across different TagType values.
   
   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.
   If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
   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>



-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5706354827

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @eschutho Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5953085110

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] migration: update tag unique constraint to (name, type) [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.

korbit-ai[bot] commented on code in PR #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r2408961842


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    try:
+        # Drop the old unique constraint on name only
+        op.drop_constraint("tag_name_key", "tag", type_="unique")
+
+        # Create new composite unique constraint on (name, type)
+        op.create_unique_constraint("uix_tag_name_type", "tag", ["name", "type"])
+    except Exception:  # noqa: S110
+        # SQLite doesn't support constraint operations well
+        pass
+
+
+def downgrade():
+    """
+    Revert to name-only unique constraint.
+    Note: This may fail if there are duplicate names with different types.
+    """

Review Comment:
   ### Downgrade fails with duplicate tag names <sub>![category Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The downgrade function does not handle the case where duplicate tag names with different types exist, which will cause the migration to fail when trying to create a single-column unique constraint.
   
   
   ###### Why this matters
   If users have created tags with the same name but different types after the upgrade, the downgrade will fail with a constraint violation, leaving the database in a broken migration state that requires manual intervention.
   
   ###### Suggested change ∙ *Feature Preview*
   Add data validation and cleanup before applying the constraint:
   
   ```python
   def downgrade():
       """
       Revert to name-only unique constraint.
       Note: This may fail if there are duplicate names with different types.
       """
       from sqlalchemy import inspect, text
       
       conn = op.get_bind()
       inspector = inspect(conn)
       
       # Check if we're using SQLite
       if conn.dialect.name == 'sqlite':
           return
       
       # Check for duplicate names before proceeding
       result = conn.execute(text(
           "SELECT name, COUNT(*) as count FROM tag GROUP BY name HAVING COUNT(*) > 1"
       ))
       duplicates = result.fetchall()
       
       if duplicates:
           raise ValueError(
               f"Cannot downgrade: Found duplicate tag names: {[row[0] for row in duplicates]}. "
               "Please resolve duplicates manually before downgrading."
           )
       
       # Drop the composite unique constraint if it exists
       existing_constraints = [c['name'] for c in inspector.get_unique_constraints('tag')]
       if 'uix_tag_name_type' in existing_constraints:
           op.drop_constraint("uix_tag_name_type", "tag", type_="unique")
       
       # Recreate the old unique constraint on name only
       op.create_unique_constraint("tag_name_key", "tag", ["name"])
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/643e1e7c-c1a1-4090-a854-c9070641a14c/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/643e1e7c-c1a1-4090-a854-c9070641a14c?what_not_true=true)  [![Not in Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/643e1e7c-c1a1-4090-a854-c9070641a14c?what_out_of_scope=true) [![Not in coding standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/643e1e7c-c1a1-4090-a854-c9070641a14c?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/643e1e7c-c1a1-4090-a854-c9070641a14c)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:478f0bf5-c1cc-4ff4-8a94-a00a0d2e86b1 -->
   
   
   [](478f0bf5-c1cc-4ff4-8a94-a00a0d2e86b1)



-- 
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] migration: update tag unique constraint to (name, type) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-3374627364

   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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6070746744

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-3392681267

   reviewed with claude, main thing in my prompt was asking whether we should use the migration utils..
   ---
   
   The migration looks good overall. A few suggestions:
   
   ### 1. Use migration utilities
   
   Consider using `migration_utils.create_unique_constraint()` and `migration_utils.drop_unique_constraint()` instead of raw Alembic operations. These handle SQLite compatibility and idempotency.
   
   Reference: `superset/migrations/versions/2024-01-17_13-09_96164e3017c6_tagged_object_unique_constraint.py:84-88`
   
   ```python
   import migration_utils as utils
   
   def upgrade() -> None:
       utils.drop_unique_constraint(op, "tag_name_key", "tag")
       utils.create_unique_constraint(op, "uix_tag_name_type", "tag", ["name", "type"])
   
   def downgrade() -> None:
       utils.drop_unique_constraint(op, "uix_tag_name_type", "tag")
       utils.create_unique_constraint(op, "tag_name_key", "tag", ["name"])
   ```
   
   ### 2. Consider downgrade edge case
   
   If duplicate tag names exist after upgrade, downgrade will fail when recreating the single-column constraint. Consider checking for duplicates first or documenting this limitation.
   
   Otherwise looks good to merge once these are addressed.
   


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5763312890

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @eschutho Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5704362967

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @eschutho Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6102863262

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [superset]

Posted by "eschutho (via GitHub)" <gi...@apache.org>.

eschutho commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6208505055

   @michael-s-molina Updated per your request: the migration's `down_revision` and `Revises:` docstring now point at the current alembic head on master (`c179af013f48`, add_semantic_layer_cache_version), and the branch has been rebased onto current master, so there is a single migration head again. Ready for re-review when you have a moment.


-- 
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: update tag unique constraint to (name, type) [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 #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3408398852


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():

Review Comment:
   **Suggestion:** Add an explicit return type annotation to this new migration function (for example, annotate it as returning no value). [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   The rule requires newly added Python functions to be fully typed, including return annotations. This new migration function has no return type hint, so the suggestion correctly identifies a real violation.
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7a309866bd7a4a299bd19861612f3f82&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7a309866bd7a4a299bd19861612f3f82&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/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 62:62
   **Comment:**
   	*Custom Rule: Add an explicit return type annotation to this new migration function (for example, annotate it as returning no value).
   
   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%2F35542&comment_hash=5f60404d6b6d9f8c030370cfb6dd2891762c7aee3b151454fe1560a3a4618eae&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=5f60404d6b6d9f8c030370cfb6dd2891762c7aee3b151454fe1560a3a4618eae&reaction=dislike'>👎</a>



##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4

Review Comment:
   **Suggestion:** Add an inline class docstring describing the purpose of this new enum class. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   The rule requires newly added classes to include docstrings. This enum class does not have a docstring immediately under the class definition, so the suggestion is a real rule violation.
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=13d367610daf458da34f6b1ebafeb367&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=13d367610daf458da34f6b1ebafeb367&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/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 37:42
   **Comment:**
   	*Custom Rule: Add an inline class docstring describing the purpose of this new enum class.
   
   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%2F35542&comment_hash=1fafe40b840d48d179b93478b63b1f5880478ad8595715a98c2bd5c1ccf76032&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=1fafe40b840d48d179b93478b63b1f5880478ad8595715a98c2bd5c1ccf76032&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: update tag unique constraint to (name, type) [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6208646090

   ## CodeAnt PR Risk: High Risk
   
   - The PR should not merge yet: allowing duplicate tag names makes existing name-only deletion paths capable of deleting both custom and system tags.
   - The migration drops a hard-coded tag_name_key; databases that named the original unique index differently can fail superset db upgrade.
   - The duplicate cleanup can delete referenced duplicate NULL-name tags, causing the migration to fail where foreign keys are enforced.
   
   **Assessed commit:** `dad96b22fa76`


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6209342913

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/35542?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 65.01%. Comparing base ([`5c5061b`](https://app.codecov.io/gh/apache/superset/commit/5c5061b1c8e41c0c55119d67ccefe42d869b9afc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7e32041`](https://app.codecov.io/gh/apache/superset/commit/7e32041712feb50e7d1ecf1d59419e89fc97652f?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   #35542      +/-   ##
   ==========================================
   - Coverage   65.14%   65.01%   -0.13%     
   ==========================================
     Files        3047     3047              
     Lines      199499   199500       +1     
     Branches    46549    46549              
   ==========================================
   - Hits       129956   129713     -243     
   - Misses      66621    66856     +235     
   - Partials     2922     2931       +9     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/35542/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/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.97% <100.00%> (+<0.01%)` | :arrow_up: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.93% <100.00%> (-0.01%)` | :arrow_down: |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.72% <100.00%> (+<0.01%)` | :arrow_up: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.01% <100.00%> (-0.21%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/35542/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   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/35542?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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6281412613

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6022156905

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-6455867144

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] migration: update tag unique constraint to (name, type) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on code in PR #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r2408973048


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    try:
+        # Drop the old unique constraint on name only
+        op.drop_constraint("tag_name_key", "tag", type_="unique")
+
+        # Create new composite unique constraint on (name, type)
+        op.create_unique_constraint("uix_tag_name_type", "tag", ["name", "type"])
+    except Exception:  # noqa: S110

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Avoid catching blind exceptions</b></div>
   <div id="fix">
   
   Catching blind `Exception` is discouraged (`BLE001`). Consider catching more specific exceptions like `OperationalError` or at minimum, log the exception details.
   </div>
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    from alembic import op
    +from sqlalchemy.exc import OperationalError
    +import logging
   
    # revision identifiers, used by Alembic.
    revision = "b54f3bd8e69"
    @@ -43,8 +45,9 @@
            # Create new composite unique constraint on (name, type)
            op.create_unique_constraint("uix_tag_name_type", "tag", ["name", "type"])
    -    except Exception:  # noqa: S110
    +    except OperationalError:
            # SQLite doesn't support constraint operations well
    +        logging.info("SQLite detected, skipping constraint modification")
            pass
   ```
   
   </div>
   </details>
   </div>
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/35542#issuecomment-3374627364>#e7b0fc</a></i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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: update tag unique constraint to (name, type) [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 #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3408404159


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    bind = op.get_bind()
+
+    # Reflect the current database state to get existing tables
+    metadata.reflect(bind=bind)
+
+    # Delete duplicate tags if any, keeping the one with the lowest ID
+    min_id_subquery = (
+        select(
+            [
+                func.min(tag_table.c.id).label("min_id"),
+                tag_table.c.name,
+                tag_table.c.type,
+            ]
+        )
+        .group_by(
+            tag_table.c.name,
+            tag_table.c.type,
+        )
+        .alias("min_ids")
+    )
+
+    delete_query = tag_table.delete().where(
+        tag_table.c.id.notin_(select([min_id_subquery.c.min_id]))
+    )
+
+    bind.execute(delete_query)
+
+    # Drop the old unique constraint on name only
+    utils.drop_unique_constraint(op, old_constraint_name, table_name)

Review Comment:
   **Suggestion:** The migration hardcodes the old unique constraint name as `tag_name_key`, but the original `name` uniqueness was created from `unique=True` and its physical name is backend-dependent (for example MySQL commonly uses `name`). Dropping a non-existent constraint name will raise during `superset db upgrade` on those databases. Resolve the existing unique constraint name via inspector (matching column `name`) before dropping, instead of assuming a fixed identifier. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ DB migrations fail on MySQL metadata backends.
   - ❌ Docker init scripts running `superset db upgrade` abort.
   - ⚠️ Tag unique constraint change never applied on affected DBs.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Initialize Superset using a MySQL metadata database and start it via the provided
   Docker init script `docker/docker-init.sh:46-48`, which runs `superset db upgrade` to
   apply Alembic migrations.
   
   2. During `superset db upgrade`, Alembic processes migration
   `b54f3bd8e69_update_tag_unique_constraint` and executes `upgrade()` in
   `superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:62-101`.
   
   3. The existing `tag` table and its unique index on `name` were originally created in
   migration `2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py:73-81` with `name =
   Column(String(250), unique=True)` and no explicit constraint name, leaving the physical
   unique index name up to the database backend (PostgreSQL uses `tag_name_key`, MySQL
   typically uses the column name, e.g. `name`).
   
   4. In `upgrade()`, the code calls `utils.drop_unique_constraint(op, old_constraint_name,
   table_name)` at `2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:95-96`, and
   `drop_unique_constraint` in `superset/migrations/migration_utils.py:48-59` blindly issues
   a `DROP CONSTRAINT`/`DROP INDEX` for the provided name; on MySQL, where no constraint
   named `tag_name_key` exists for the `tag` table, this raises a database error and causes
   `superset db upgrade` (and thus application startup) to fail.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=af9a34f661304a6291ba49caa55a2c31&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=af9a34f661304a6291ba49caa55a2c31&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/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py
   **Line:** 56:96
   **Comment:**
   	*Api Mismatch: The migration hardcodes the old unique constraint name as `tag_name_key`, but the original `name` uniqueness was created from `unique=True` and its physical name is backend-dependent (for example MySQL commonly uses `name`). Dropping a non-existent constraint name will raise during `superset db upgrade` on those databases. Resolve the existing unique constraint name via inspector (matching column `name`) before dropping, instead of assuming a fixed identifier.
   
   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%2F35542&comment_hash=2a962011ae69c4376751c019c074a33d2bb1cc28b8c603893fb6c11cdfd14990&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35542&comment_hash=2a962011ae69c4376751c019c074a33d2bb1cc28b8c603893fb6c11cdfd14990&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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5719098259

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @eschutho Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: update tag unique constraint to (name, type) [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35542:
URL: https://github.com/apache/superset/pull/35542#issuecomment-5786545066

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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

Re: [PR] fix: update tag unique constraint to (name, type) [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #35542:
URL: https://github.com/apache/superset/pull/35542#discussion_r3825413825


##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    bind = op.get_bind()
+
+    # Reflect the current database state to get existing tables
+    metadata.reflect(bind=bind)
+
+    # Delete duplicate tags if any, keeping the one with the lowest ID
+    min_id_subquery = (
+        select(
+            [
+                func.min(tag_table.c.id).label("min_id"),
+                tag_table.c.name,
+                tag_table.c.type,
+            ]
+        )
+        .group_by(
+            tag_table.c.name,
+            tag_table.c.type,
+        )
+        .alias("min_ids")
+    )
+
+    delete_query = tag_table.delete().where(
+        tag_table.c.id.notin_(select([min_id_subquery.c.min_id]))
+    )
+
+    bind.execute(delete_query)

Review Comment:
   This can delete an existing tag before the constraint change. `name` is nullable, and the old single-column unique constraint permits multiple NULL names; two such tags with the same type are grouped here even though the new composite constraint also permits them. If either tag is referenced, the migration either fails on the foreign key or loses its association. Could this skip NULL names (or preserve/remap references) instead of deleting them?



##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    try:
+        # Drop the old unique constraint on name only
+        op.drop_constraint("tag_name_key", "tag", type_="unique")
+
+        # Create new composite unique constraint on (name, type)
+        op.create_unique_constraint("uix_tag_name_type", "tag", ["name", "type"])
+    except Exception:  # noqa: S110
+        # SQLite doesn't support constraint operations well
+        pass
+
+
+def downgrade():
+    """
+    Revert to name-only unique constraint.
+    Note: This may fail if there are duplicate names with different types.
+    """

Review Comment:
   Agreed—the upgrade enables same-name tags across types, so recreating the name-only constraint during downgrade can fail and leave the migration half-applied. Should the downgrade reject that state before dropping the composite constraint?



##########
superset/migrations/versions/2025-10-06_16-05_b54f3bd8e69_update_tag_unique_constraint.py:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""update_tag_unique_constraint
+
+Revision ID: b54f3bd8e69
+Revises: c233f5365c9e
+Create Date: 2025-10-06 16:05:00.000000
+
+"""
+
+import enum
+
+import migration_utils as utils
+from alembic import op
+from sqlalchemy import Column, Enum, Integer, MetaData, String, Table, Text
+from sqlalchemy.sql import func, select
+
+# revision identifiers, used by Alembic.
+revision = "b54f3bd8e69"
+down_revision = "c233f5365c9e"
+
+
+class TagType(enum.Enum):
+    # pylint: disable=invalid-name
+    custom = 1
+    type = 2
+    owner = 3
+    favorited_by = 4
+
+
+# Define the tag table structure for data operations
+metadata = MetaData()
+tag_table = Table(
+    "tag",
+    metadata,
+    Column("id", Integer, primary_key=True),
+    Column("name", String(250)),
+    Column("type", Enum(TagType)),
+    Column("description", Text),
+)
+
+old_constraint_name = "tag_name_key"
+new_constraint_name = "uix_tag_name_type"
+table_name = "tag"
+new_constraint_columns = ["name", "type"]
+
+
+def upgrade():
+    """
+    Change tag unique constraint from name only to (name, type) composite.
+    This allows the same tag name to exist with different types (e.g., 'type:dashboard'
+    can be both a system tag with type='type' and a custom tag with type='custom').
+    """
+    bind = op.get_bind()
+
+    # Reflect the current database state to get existing tables
+    metadata.reflect(bind=bind)
+
+    # Delete duplicate tags if any, keeping the one with the lowest ID
+    min_id_subquery = (
+        select(
+            [
+                func.min(tag_table.c.id).label("min_id"),
+                tag_table.c.name,
+                tag_table.c.type,
+            ]
+        )
+        .group_by(
+            tag_table.c.name,
+            tag_table.c.type,
+        )
+        .alias("min_ids")
+    )
+
+    delete_query = tag_table.delete().where(
+        tag_table.c.id.notin_(select([min_id_subquery.c.min_id]))
+    )
+
+    bind.execute(delete_query)
+
+    # Drop the old unique constraint on name only
+    utils.drop_unique_constraint(op, old_constraint_name, table_name)
+

Review Comment:
   Agreed—the original name-only uniqueness was unnamed, so this PostgreSQL-specific identifier is absent on MySQL and SQLite and the migration aborts before the replacement constraint is created. Could this discover the existing name-based unique constraint/index per dialect before dropping 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