codeant-ai-for-open-source[bot] commented on code in PR #40128:
URL: https://github.com/apache/superset/pull/40128#discussion_r3500155870
##########
superset/daos/dashboard.py:
##########
@@ -245,13 +245,26 @@ def get_dashboard_and_datasets_changed_on( # pylint:
disable=invalid-name
@staticmethod
def validate_slug_uniqueness(slug: str) -> bool:
+ # The ``SoftDeleteMixin`` listener auto-appends ``deleted_at IS NULL``
+ # to this query, so soft-deleted rows are correctly ignored on
+ # PostgreSQL and MySQL 8.0+ where slug uniqueness is enforced by a
+ # partial index (``ix_dashboards_active_slug``). On SQLite, MariaDB,
+ # and MySQL <8.0 the database retains a full unique constraint on
+ # ``slug``; a soft-deleted row will still block insertion of a new
+ # row with the same slug at flush time, even though this check
+ # passes. The fallback constraint is documented in UPDATING.md and
+ # in the 9e1f3b8c4d2a migration.
if not slug:
return True
dashboard_query = db.session.query(Dashboard).filter(Dashboard.slug ==
slug)
return not db.session.query(dashboard_query.exists()).scalar()
@staticmethod
def validate_update_slug_uniqueness(dashboard_id: int, slug: str | None)
-> bool:
+ # See ``validate_slug_uniqueness`` for the same dialect-gap note:
+ # the partial-index dialects (Postgres / MySQL 8.0+) match this
+ # application-level check; the full-constraint dialects can still
+ # surface an IntegrityError when the colliding row is soft-deleted.
Review Comment:
**Suggestion:** The new comment says partial-index behavior applies to MySQL
8.0+, but the migration actually enables this only on MySQL 8.0.13+ (and
excludes MariaDB). Update the comment to the exact supported version range so
future maintainers do not make incorrect assumptions about slug-conflict
behavior on older MySQL patch releases. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor π§Ή</summary>
```mdx
β οΈ MySQL 8.0.0β8.0.12 behavior misdocumented in DAO comments.
β οΈ MariaDB deployments misled about slug index semantics.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Open
`superset/migrations/versions/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py`
and inspect `_mysql_supports_functional_index` at lines 8β26, which
explicitly returns
True only for MySQL `server_version_info >= (8, 0, 13)` and returns False
for MariaDB
(lines 18β26).
2. In the same migration file, inspect
`_replace_slug_constraint_with_partial_index` at
lines 51β75, where the MySQL branch is guarded by `elif dialect == "mysql"
and
_mysql_supports_functional_index(bind):`, meaning the partial active-slug
index is created
only on MySQL 8.0.13+ and never on MariaDB.
3. Open `superset/daos/dashboard.py` and review `validate_slug_uniqueness`
at lines 27β37,
where the comment states "PostgreSQL and MySQL 8.0+ where slug uniqueness is
enforced by a
partial index", and lists SQLite, MariaDB, and MySQL <8.0 as retaining a
full unique
constraint (lines 31β37), implying all MySQL 8.0.x support the partial index.
4. In the same DAO file, review `validate_update_slug_uniqueness` at lines
45β48
(including the snippet at 265β267), which repeats "the partial-index
dialects (Postgres /
MySQL 8.0+)" language, confirming that DAO-layer documentation asserts
broader MySQL
support than the migration actually provides (8.0.13+ only), creating a
comment/migration
mismatch about slug-conflict behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ed363aaec7b348c98c3a40613ee71a60&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=ed363aaec7b348c98c3a40613ee71a60&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/daos/dashboard.py
**Line:** 265:267
**Comment:**
*Comment Mismatch: The new comment says partial-index behavior applies
to MySQL 8.0+, but the migration actually enables this only on MySQL 8.0.13+
(and excludes MariaDB). Update the comment to the exact supported version range
so future maintainers do not make incorrect assumptions about slug-conflict
behavior on older MySQL patch releases.
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=1ff0ebf8b6198edf4e50e55e25ddfe86f5802ec519b519d32ec092d264d95b43&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=1ff0ebf8b6198edf4e50e55e25ddfe86f5802ec519b519d32ec092d264d95b43&reaction=dislike'>π</a>
##########
superset/models/dashboard.py:
##########
@@ -141,7 +145,13 @@ class Dashboard(CoreDashboard, AuditMixinNullable,
ImportExportMixin):
certified_by = Column(Text)
certification_details = Column(Text)
json_metadata = Column(utils.MediumText())
- slug = Column(String(255), unique=True)
+ # Slug uniqueness is enforced via a partial unique index
+ # (``ix_dashboards_active_slug WHERE deleted_at IS NULL``) on
+ # PostgreSQL and MySQL 8.0+, so soft-deleted rows do not reserve
+ # their slug. SQLite and MySQL <8.0 keep the original full unique
+ # constraint via the migration; on those dialects slug reservation
+ # persists across soft-delete. See the 9e1f3b8c4d2a migration for details.
Review Comment:
**Suggestion:** The added model comment also claims MySQL 8.0+ support for
the partial unique slug index, but the migration enforces a stricter MySQL
8.0.13+ requirement. Align this comment with the migration rule to avoid
misleading documentation in the model layer. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor π§Ή</summary>
```mdx
β οΈ Model docs misstate MySQL version requirement for partial index.
β οΈ Operators may misconfigure upgrades on unsupported MySQL patch levels.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Open
`superset/migrations/versions/2026-05-08_12-05_9e1f3b8c4d2a_add_deleted_at_to_dashboards.py`
and examine `_mysql_supports_functional_index` at lines 8β26, which requires
`(bind.dialect.server_version_info or ()) >= (8, 0, 13)` and returns False
for MariaDB via
`getattr(bind.dialect, "is_mariadb", False)` (lines 18β26).
2. In the same migration file, inspect
`_replace_slug_constraint_with_partial_index` at
lines 51β75, where the MySQL path is guarded by `elif dialect == "mysql" and
_mysql_supports_functional_index(bind):`, ensuring the partial active-slug
index is only
created for MySQL 8.0.13+ and not for MySQL 8.0.0β8.0.12 nor MariaDB.
3. Open `superset/models/dashboard.py` and review the `Dashboard` model
definition at
lines 19β25, specifically the comment at 150β153 stating "PostgreSQL and
MySQL 8.0+, so
soft-deleted rows do not reserve their slug. SQLite and MySQL <8.0 keep the
original full
unique constraint via the migration", which documents partial-index behavior
as applying
to all MySQL 8.0+.
4. Compare the migrationβs stricter MySQL 8.0.13+ requirement to the modelβs
"MySQL 8.0+"
phrasing: on MySQL 8.0.0β8.0.12 the database still uses the full unique slug
constraint,
but the model comment claims soft-deleted rows do not reserve their slug,
creating
misleading documentation about supported MySQL versions for the partial
unique index.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cfb7a4a5f1aa41a1ad0e20574c15f799&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=cfb7a4a5f1aa41a1ad0e20574c15f799&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/models/dashboard.py
**Line:** 150:153
**Comment:**
*Comment Mismatch: The added model comment also claims MySQL 8.0+
support for the partial unique slug index, but the migration enforces a
stricter MySQL 8.0.13+ requirement. Align this comment with the migration rule
to avoid misleading documentation in the model layer.
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=a4de0d5e2b7454c81494e661488f9cad3e4535a32532363c82b9a09f208fcf33&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=a4de0d5e2b7454c81494e661488f9cad3e4535a32532363c82b9a09f208fcf33&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]
