codeant-ai-for-open-source[bot] commented on code in PR #40128:
URL: https://github.com/apache/superset/pull/40128#discussion_r3500130351
##########
superset/dashboards/filters.py:
##########
@@ -265,3 +266,38 @@ def apply(self, query: Query, value: Any) -> Query:
if value is False:
return query.filter(and_(Dashboard.created_by_fk.is_(None)))
return query
+
+
+class DashboardDeletedStateFilter( # pylint: disable=too-few-public-methods
+ BaseDeletedStateFilter
+):
+ """Rison filter for the GET list that exposes soft-deleted dashboards.
+
+ Soft-deleted rows are additionally scoped to the **restore audience**:
+ only the dashboard's owners (or admins) may enumerate them. This mirrors
+ ``RestoreDashboardCommand``'s ``raise_for_ownership`` check, so a
+ read-access non-owner (who can see the dashboard via published-datasource
+ access or dashboard RBAC) cannot list soft-deleted dashboards they could
+ never restore. Live rows are unaffected β they keep their normal
+ ``DashboardAccessFilter`` visibility.
+ """
+
+ arg_name = "dashboard_deleted_state"
Review Comment:
**Suggestion:** Add an explicit type annotation to this class attribute so
the new filterβs configuration is fully typed. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This new class attribute is a plain untyped string assignment, and the rule
requires type hints on relevant variables that can be annotated. The suggestion
correctly identifies a real omission.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b20e0afa31a142cabea6eaeb373d5fc6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b20e0afa31a142cabea6eaeb373d5fc6&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/dashboards/filters.py
**Line:** 285:285
**Comment:**
*Custom Rule: Add an explicit type annotation to this class attribute
so the new filterβs configuration is fully typed.
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%2F40128&comment_hash=ae7cdd49c830d5538442d163ae55e2d416e7f9689e1aa10a7890f276c34dbca3&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=ae7cdd49c830d5538442d163ae55e2d416e7f9689e1aa10a7890f276c34dbca3&reaction=dislike'>π</a>
##########
superset/dashboards/filters.py:
##########
@@ -265,3 +266,38 @@ def apply(self, query: Query, value: Any) -> Query:
if value is False:
return query.filter(and_(Dashboard.created_by_fk.is_(None)))
return query
+
+
+class DashboardDeletedStateFilter( # pylint: disable=too-few-public-methods
+ BaseDeletedStateFilter
+):
+ """Rison filter for the GET list that exposes soft-deleted dashboards.
+
+ Soft-deleted rows are additionally scoped to the **restore audience**:
+ only the dashboard's owners (or admins) may enumerate them. This mirrors
+ ``RestoreDashboardCommand``'s ``raise_for_ownership`` check, so a
+ read-access non-owner (who can see the dashboard via published-datasource
+ access or dashboard RBAC) cannot list soft-deleted dashboards they could
+ never restore. Live rows are unaffected β they keep their normal
+ ``DashboardAccessFilter`` visibility.
+ """
+
+ arg_name = "dashboard_deleted_state"
+ model = Dashboard
Review Comment:
**Suggestion:** Add a type annotation to this model reference attribute to
satisfy the rule requiring type hints on relevant variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly added class-level variable that can be annotated but is left
untyped. That matches the custom rule requiring type hints for relevant
variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7c13bf2443a4450495f65befa90f2e15&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7c13bf2443a4450495f65befa90f2e15&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/dashboards/filters.py
**Line:** 286:286
**Comment:**
*Custom Rule: Add a type annotation to this model reference attribute
to satisfy the rule requiring type hints on relevant variables.
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%2F40128&comment_hash=4392642392b12de30a1657088959eed47a7f001bfdade3c8671eb1f8097b274b&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=4392642392b12de30a1657088959eed47a7f001bfdade3c8671eb1f8097b274b&reaction=dislike'>π</a>
##########
superset/migrations/versions/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py:
##########
@@ -0,0 +1,201 @@
+# 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.
+"""Add deleted_at + partial unique slug index for soft-delete.
+
+Adds to the ``dashboards`` table:
+- a nullable ``deleted_at`` column for soft-delete state
+- an index ``ix_dashboards_deleted_at`` for the visibility-filter listener
+- a partial unique index ``ix_dashboards_active_slug`` enforcing slug
+ uniqueness only among active (non-soft-deleted) rows
+
+Drops:
+- the existing full unique constraint on ``slug`` (named
+ ``idx_unique_slug``, created in migration 1a48a5411020)
+
+The constraint change makes the ``slug`` field reusable after soft-delete:
+soft-deleted rows no longer reserve their slug for the lifetime of the
+row. ``RestoreDashboardCommand`` handles the reverse case (restoring a
+dashboard whose slug has since been claimed by another active row) with
+an explicit conflict error. See UPDATING.md for the user-facing change.
+
+Dialect support for the partial index:
+- PostgreSQL: native ``WHERE deleted_at IS NULL`` partial index
+- MySQL 8.0+: functional index over
+ ``(CASE WHEN deleted_at IS NULL THEN slug END)``
+- MySQL <8.0: keeps the original full unique constraint (documented
+ limitation; functional indexes are not supported on these versions)
+- SQLite: keeps the original full unique constraint (column-level
+ ``UNIQUE`` cannot be dropped without recreating the table, which is
+ not worth the migration complexity for a test-only dialect). Tests
+ that need to verify the partial-index behaviour run only on
+ PostgreSQL and MySQL 8+.
+
+Revision ID: 9e1f3b8c4d2a
+Revises: 78a40c08b4be
+Create Date: 2026-05-08 12:05:00.000000
+"""
+
+from alembic import op
+from sqlalchemy import Column, DateTime
+from sqlalchemy.engine import Connection
+
+from superset.migrations.shared.utils import (
+ add_columns,
+ create_index,
+ drop_columns,
+ drop_index,
+ table_has_index,
+)
+
+# revision identifiers, used by Alembic.
+revision = "9e1f3b8c4d2a"
+down_revision = "78a40c08b4be"
+
+TABLE_NAME = "dashboards"
+DELETED_AT_INDEX_NAME = f"ix_{TABLE_NAME}_deleted_at"
+PARTIAL_SLUG_INDEX_NAME = f"ix_{TABLE_NAME}_active_slug"
+# The original full unique constraint on ``slug`` was created with an
+# explicit name in migration 1a48a5411020 (2015-12-04). Same name on
+# PostgreSQL (constraint) and MySQL (index).
+LEGACY_SLUG_INDEX_NAME = "idx_unique_slug"
+
+
+def _mysql_supports_functional_index(bind: Connection) -> bool:
+ """Return True iff the connected MySQL is 8.0.13+ (supports functional
indexes).
+
+ MySQL added functional key parts in 8.0.13; 8.0.0β8.0.12 reject the
+ ``(CASE WHEN deleted_at IS NULL THEN slug END)`` expression at index
+ creation time, so deployments on those patch releases must keep the
+ original full slug constraint. See
+ https://dev.mysql.com/doc/mysql/8.0/en/create-index.html for the
+ 8.0.13 minimum.
+
+ Excludes MariaDB even at server version ``>= (10, x)`` because MariaDB
+ reports through the same ``server_version_info`` attribute but uses
+ different functional-index semantics around ``CASE`` expressions.
+ Uses SQLAlchemy's parsed ``server_version_info`` rather than ``SELECT
+ VERSION()`` to avoid an extra round-trip and brittle string parsing.
+ """
+ if getattr(bind.dialect, "is_mariadb", False):
+ return False
+ return (bind.dialect.server_version_info or ()) >= (8, 0, 13)
+
+
+def upgrade() -> None:
+ bind = op.get_bind()
+ _add_deleted_at_column()
+ _replace_slug_constraint_with_partial_index(bind)
+
+
+def downgrade() -> None:
+ bind = op.get_bind()
+ _restore_slug_constraint(bind)
+ _drop_deleted_at_column()
+
+
+def _add_deleted_at_column() -> None:
+ add_columns(TABLE_NAME, Column("deleted_at", DateTime(), nullable=True))
+ create_index(TABLE_NAME, DELETED_AT_INDEX_NAME, ["deleted_at"])
+
+
+def _drop_deleted_at_column() -> None:
+ drop_index(TABLE_NAME, DELETED_AT_INDEX_NAME)
+ drop_columns(TABLE_NAME, "deleted_at")
+
+
+def _replace_slug_constraint_with_partial_index(bind: Connection) -> None:
+ """Swap the full UNIQUE on ``slug`` for a partial index where supported.
+
+ The original constraint is named ``idx_unique_slug`` from migration
+ 1a48a5411020 β same name on PostgreSQL (constraint) and MySQL (index).
+
+ SQLite and MySQL <8.0 are no-ops here: they keep the original full
+ unique constraint. See the module docstring for the rationale.
+ """
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"DROP CONSTRAINT IF EXISTS {LEGACY_SLUG_INDEX_NAME}"
+ )
+ # Some installations may have the unique enforced as a plain
+ # index rather than a constraint. Both DROPs are IF EXISTS, so
+ # whichever path applies cleans up.
+ op.execute(f"DROP INDEX IF EXISTS {LEGACY_SLUG_INDEX_NAME}")
+ op.execute(
+ f"CREATE UNIQUE INDEX {PARTIAL_SLUG_INDEX_NAME} "
+ f"ON {TABLE_NAME} (slug) WHERE deleted_at IS NULL"
+ )
+ elif dialect == "mysql" and _mysql_supports_functional_index(bind):
+ # Create the functional replacement BEFORE dropping the legacy unique
+ # index. MySQL autocommits each DDL statement (unlike PostgreSQL's
+ # transactional DDL above, where a failed CREATE rolls back the DROP),
+ # so a drop-then-create ordering would leave the table with no slug
+ # uniqueness if the CREATE failed. Creating first keeps the stricter
+ # existing uniqueness in place until the replacement is confirmed.
+ # Both statements are guarded by ``table_has_index`` because MySQL has
+ # no ``IF [NOT] EXISTS`` for indexes and DDL autocommits: an unguarded
+ # run on a table missing the legacy index (it was created inside
+ # ``try/except: pass`` in 2015's ``1a48a5411020``) would fail AFTER
+ # the partial index was committed, wedging the migration β the re-run
+ # would then die on the duplicate partial index. The guards make the
+ # migration re-runnable from any partial state.
+ if not table_has_index(TABLE_NAME, PARTIAL_SLUG_INDEX_NAME):
+ op.execute(
+ f"CREATE UNIQUE INDEX {PARTIAL_SLUG_INDEX_NAME} "
+ f"ON {TABLE_NAME} ((CASE WHEN deleted_at IS NULL THEN slug
END))"
+ )
+ if table_has_index(TABLE_NAME, LEGACY_SLUG_INDEX_NAME):
+ op.execute(f"ALTER TABLE {TABLE_NAME} DROP INDEX
{LEGACY_SLUG_INDEX_NAME}")
+
+
+def _restore_slug_constraint(bind: Connection) -> None:
+ """Restore the full UNIQUE on ``slug`` from the partial index.
+
+ Symmetric counterpart to ``_replace_slug_constraint_with_partial_index``.
+ No-op on dialects that never received the partial index.
+
+ Pre-condition: each value of ``slug`` (other than NULL) must appear at
+ most once across the entire ``dashboards`` table. The partial-index
+ window allowed an active row and a soft-deleted row to share a slug;
+ rebuilding the full unique constraint will abort with a
+ ``UniqueViolation`` if any such pair still exists. Before downgrading,
+ hard-delete the soft-deleted duplicates (or rename one side of each
+ pair) so the constraint can be added cleanly.
+ """
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ op.execute(f"DROP INDEX IF EXISTS {PARTIAL_SLUG_INDEX_NAME}")
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"ADD CONSTRAINT {LEGACY_SLUG_INDEX_NAME} UNIQUE (slug)"
+ )
+ elif dialect == "mysql" and _mysql_supports_functional_index(bind):
+ # Symmetric to the upgrade: add the full unique index before dropping
+ # the partial one, so a failed ADD leaves the partial uniqueness intact
+ # rather than no uniqueness (MySQL autocommits each DDL statement).
+ # Guarded like the upgrade so the downgrade is re-runnable from any
+ # partial state β including a deployment that migrated on a pre-8.0.13
+ # server (no-op upgrade branch, legacy index still present) and later
+ # upgraded the server, where the unguarded ADD would collide.
+ if not table_has_index(TABLE_NAME, LEGACY_SLUG_INDEX_NAME):
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"ADD UNIQUE INDEX {LEGACY_SLUG_INDEX_NAME} (slug)"
+ )
+ if table_has_index(TABLE_NAME, PARTIAL_SLUG_INDEX_NAME):
+ op.execute(f"ALTER TABLE {TABLE_NAME} DROP INDEX
{PARTIAL_SLUG_INDEX_NAME}")
Review Comment:
**Suggestion:** Use the shared migration index-drop helper for dropping the
partial slug index in MySQL downgrade paths rather than executing raw SQL.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This MySQL downgrade path uses raw SQL to drop an index even though the
shared migration helper `drop_index` is available and imported. The suggestion
correctly identifies a helper-usage violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8b1e96e1c6d84883a7121c63565119ed&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8b1e96e1c6d84883a7121c63565119ed&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/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py
**Line:** 201:201
**Comment:**
*Custom Rule: Use the shared migration index-drop helper for dropping
the partial slug index in MySQL downgrade paths rather than executing raw SQL.
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%2F40128&comment_hash=4d0247e78f8ca45bcf4c15ed5c3a60c10f08585d04564c9d4adede0190eae53d&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=4d0247e78f8ca45bcf4c15ed5c3a60c10f08585d04564c9d4adede0190eae53d&reaction=dislike'>π</a>
##########
superset/migrations/versions/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py:
##########
@@ -0,0 +1,201 @@
+# 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.
+"""Add deleted_at + partial unique slug index for soft-delete.
+
+Adds to the ``dashboards`` table:
+- a nullable ``deleted_at`` column for soft-delete state
+- an index ``ix_dashboards_deleted_at`` for the visibility-filter listener
+- a partial unique index ``ix_dashboards_active_slug`` enforcing slug
+ uniqueness only among active (non-soft-deleted) rows
+
+Drops:
+- the existing full unique constraint on ``slug`` (named
+ ``idx_unique_slug``, created in migration 1a48a5411020)
+
+The constraint change makes the ``slug`` field reusable after soft-delete:
+soft-deleted rows no longer reserve their slug for the lifetime of the
+row. ``RestoreDashboardCommand`` handles the reverse case (restoring a
+dashboard whose slug has since been claimed by another active row) with
+an explicit conflict error. See UPDATING.md for the user-facing change.
+
+Dialect support for the partial index:
+- PostgreSQL: native ``WHERE deleted_at IS NULL`` partial index
+- MySQL 8.0+: functional index over
+ ``(CASE WHEN deleted_at IS NULL THEN slug END)``
+- MySQL <8.0: keeps the original full unique constraint (documented
+ limitation; functional indexes are not supported on these versions)
+- SQLite: keeps the original full unique constraint (column-level
+ ``UNIQUE`` cannot be dropped without recreating the table, which is
+ not worth the migration complexity for a test-only dialect). Tests
+ that need to verify the partial-index behaviour run only on
+ PostgreSQL and MySQL 8+.
+
+Revision ID: 9e1f3b8c4d2a
+Revises: 78a40c08b4be
+Create Date: 2026-05-08 12:05:00.000000
+"""
+
+from alembic import op
+from sqlalchemy import Column, DateTime
+from sqlalchemy.engine import Connection
+
+from superset.migrations.shared.utils import (
+ add_columns,
+ create_index,
+ drop_columns,
+ drop_index,
+ table_has_index,
+)
+
+# revision identifiers, used by Alembic.
+revision = "9e1f3b8c4d2a"
+down_revision = "78a40c08b4be"
+
+TABLE_NAME = "dashboards"
+DELETED_AT_INDEX_NAME = f"ix_{TABLE_NAME}_deleted_at"
+PARTIAL_SLUG_INDEX_NAME = f"ix_{TABLE_NAME}_active_slug"
+# The original full unique constraint on ``slug`` was created with an
+# explicit name in migration 1a48a5411020 (2015-12-04). Same name on
+# PostgreSQL (constraint) and MySQL (index).
+LEGACY_SLUG_INDEX_NAME = "idx_unique_slug"
+
+
+def _mysql_supports_functional_index(bind: Connection) -> bool:
+ """Return True iff the connected MySQL is 8.0.13+ (supports functional
indexes).
+
+ MySQL added functional key parts in 8.0.13; 8.0.0β8.0.12 reject the
+ ``(CASE WHEN deleted_at IS NULL THEN slug END)`` expression at index
+ creation time, so deployments on those patch releases must keep the
+ original full slug constraint. See
+ https://dev.mysql.com/doc/mysql/8.0/en/create-index.html for the
+ 8.0.13 minimum.
+
+ Excludes MariaDB even at server version ``>= (10, x)`` because MariaDB
+ reports through the same ``server_version_info`` attribute but uses
+ different functional-index semantics around ``CASE`` expressions.
+ Uses SQLAlchemy's parsed ``server_version_info`` rather than ``SELECT
+ VERSION()`` to avoid an extra round-trip and brittle string parsing.
+ """
+ if getattr(bind.dialect, "is_mariadb", False):
+ return False
+ return (bind.dialect.server_version_info or ()) >= (8, 0, 13)
+
+
+def upgrade() -> None:
+ bind = op.get_bind()
+ _add_deleted_at_column()
+ _replace_slug_constraint_with_partial_index(bind)
+
+
+def downgrade() -> None:
+ bind = op.get_bind()
+ _restore_slug_constraint(bind)
+ _drop_deleted_at_column()
+
+
+def _add_deleted_at_column() -> None:
+ add_columns(TABLE_NAME, Column("deleted_at", DateTime(), nullable=True))
+ create_index(TABLE_NAME, DELETED_AT_INDEX_NAME, ["deleted_at"])
+
+
+def _drop_deleted_at_column() -> None:
+ drop_index(TABLE_NAME, DELETED_AT_INDEX_NAME)
+ drop_columns(TABLE_NAME, "deleted_at")
+
+
+def _replace_slug_constraint_with_partial_index(bind: Connection) -> None:
+ """Swap the full UNIQUE on ``slug`` for a partial index where supported.
+
+ The original constraint is named ``idx_unique_slug`` from migration
+ 1a48a5411020 β same name on PostgreSQL (constraint) and MySQL (index).
+
+ SQLite and MySQL <8.0 are no-ops here: they keep the original full
+ unique constraint. See the module docstring for the rationale.
+ """
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"DROP CONSTRAINT IF EXISTS {LEGACY_SLUG_INDEX_NAME}"
+ )
+ # Some installations may have the unique enforced as a plain
+ # index rather than a constraint. Both DROPs are IF EXISTS, so
+ # whichever path applies cleans up.
+ op.execute(f"DROP INDEX IF EXISTS {LEGACY_SLUG_INDEX_NAME}")
+ op.execute(
+ f"CREATE UNIQUE INDEX {PARTIAL_SLUG_INDEX_NAME} "
+ f"ON {TABLE_NAME} (slug) WHERE deleted_at IS NULL"
+ )
+ elif dialect == "mysql" and _mysql_supports_functional_index(bind):
+ # Create the functional replacement BEFORE dropping the legacy unique
+ # index. MySQL autocommits each DDL statement (unlike PostgreSQL's
+ # transactional DDL above, where a failed CREATE rolls back the DROP),
+ # so a drop-then-create ordering would leave the table with no slug
+ # uniqueness if the CREATE failed. Creating first keeps the stricter
+ # existing uniqueness in place until the replacement is confirmed.
+ # Both statements are guarded by ``table_has_index`` because MySQL has
+ # no ``IF [NOT] EXISTS`` for indexes and DDL autocommits: an unguarded
+ # run on a table missing the legacy index (it was created inside
+ # ``try/except: pass`` in 2015's ``1a48a5411020``) would fail AFTER
+ # the partial index was committed, wedging the migration β the re-run
+ # would then die on the duplicate partial index. The guards make the
+ # migration re-runnable from any partial state.
+ if not table_has_index(TABLE_NAME, PARTIAL_SLUG_INDEX_NAME):
+ op.execute(
+ f"CREATE UNIQUE INDEX {PARTIAL_SLUG_INDEX_NAME} "
+ f"ON {TABLE_NAME} ((CASE WHEN deleted_at IS NULL THEN slug
END))"
+ )
+ if table_has_index(TABLE_NAME, LEGACY_SLUG_INDEX_NAME):
+ op.execute(f"ALTER TABLE {TABLE_NAME} DROP INDEX
{LEGACY_SLUG_INDEX_NAME}")
+
+
+def _restore_slug_constraint(bind: Connection) -> None:
+ """Restore the full UNIQUE on ``slug`` from the partial index.
+
+ Symmetric counterpart to ``_replace_slug_constraint_with_partial_index``.
+ No-op on dialects that never received the partial index.
+
+ Pre-condition: each value of ``slug`` (other than NULL) must appear at
+ most once across the entire ``dashboards`` table. The partial-index
+ window allowed an active row and a soft-deleted row to share a slug;
+ rebuilding the full unique constraint will abort with a
+ ``UniqueViolation`` if any such pair still exists. Before downgrading,
+ hard-delete the soft-deleted duplicates (or rename one side of each
+ pair) so the constraint can be added cleanly.
+ """
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ op.execute(f"DROP INDEX IF EXISTS {PARTIAL_SLUG_INDEX_NAME}")
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"ADD CONSTRAINT {LEGACY_SLUG_INDEX_NAME} UNIQUE (slug)"
+ )
+ elif dialect == "mysql" and _mysql_supports_functional_index(bind):
+ # Symmetric to the upgrade: add the full unique index before dropping
+ # the partial one, so a failed ADD leaves the partial uniqueness intact
+ # rather than no uniqueness (MySQL autocommits each DDL statement).
+ # Guarded like the upgrade so the downgrade is re-runnable from any
+ # partial state β including a deployment that migrated on a pre-8.0.13
+ # server (no-op upgrade branch, legacy index still present) and later
+ # upgraded the server, where the unguarded ADD would collide.
+ if not table_has_index(TABLE_NAME, LEGACY_SLUG_INDEX_NAME):
+ op.execute(
+ f"ALTER TABLE {TABLE_NAME} "
+ f"ADD UNIQUE INDEX {LEGACY_SLUG_INDEX_NAME} (slug)"
+ )
Review Comment:
**Suggestion:** Use the shared migration index-creation helper for adding
the legacy unique index where possible instead of a raw ALTER TABLE statement.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The migration imports `create_index`, but this MySQL downgrade branch uses
raw `ALTER TABLE ... ADD UNIQUE INDEX` instead of the shared helper. That is a
valid match for the custom rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8b97712ec0f04ca786d63630d9c5f82f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8b97712ec0f04ca786d63630d9c5f82f&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/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py
**Line:** 197:199
**Comment:**
*Custom Rule: Use the shared migration index-creation helper for adding
the legacy unique index where possible instead of a raw ALTER TABLE statement.
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%2F40128&comment_hash=13d5a0328094c13f582a71d0d2209864dc07c3c41014ee20295320196fe55ade&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=13d5a0328094c13f582a71d0d2209864dc07c3c41014ee20295320196fe55ade&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
