mikebridge commented on code in PR #40130:
URL: https://github.com/apache/superset/pull/40130#discussion_r3501598709
##########
superset/commands/database/delete.py:
##########
@@ -61,6 +61,28 @@ def validate(self) -> None:
report_names=",".join(report_names),
)
)
- # Check if there are datasets for this database
- if self._model.tables:
+ # Check if there are datasets for this database. ``self._model.tables``
+ # would now hide soft-deleted datasets (``SqlaTable`` inherits
+ # ``SoftDeleteMixin``, so the relationship lazy-load applies the
+ # visibility filter), letting a database whose datasets are all
+ # soft-deleted look empty and be hard-deleted while
``tables.database_id``
+ # rows still reference it. Count with the visibility filter bypassed so
+ # soft-deleted datasets still block the delete.
+ from superset.connectors.sqla.models import ( # pylint:
disable=import-outside-toplevel
+ SqlaTable,
+ )
+ from superset.extensions import ( # pylint:
disable=import-outside-toplevel
+ db,
+ )
+ from superset.models.helpers import ( # pylint:
disable=import-outside-toplevel
+ skip_visibility_filter,
+ )
+
+ with skip_visibility_filter(db.session, SqlaTable):
+ has_datasets = db.session.query(
+ db.session.query(SqlaTable.id)
+ .filter(SqlaTable.database_id == self._model_id)
+ .exists()
+ ).scalar()
+ if has_datasets:
raise DatabaseDeleteDatasetsExistFailedError()
Review Comment:
Fixed in b9f6282765 — the guard now counts live vs. any datasets separately:
a live dataset raises the existing error, while a database whose only remaining
datasets are soft-deleted raises a new
`DatabaseDeleteSoftDeletedDatasetsExistFailedError` (a subclass of the existing
one, so the `databases/api.py` handler catches it unchanged) whose message
tells the operator to restore or purge the hidden datasets first.
_— posted by Claude (AI agent) on behalf of @mikebridge_
##########
superset/commands/dataset/duplicate.py:
##########
@@ -121,7 +121,19 @@ def validate(self) -> None:
if self._base_model and self._base_model.kind != "virtual":
exceptions.append(DatasourceTypeInvalidError())
- if DatasetDAO.find_one_or_none(table_name=duplicate_name):
+ # Use the shared uniqueness check (same as create/update) rather than a
+ # name-only filtered lookup: it scopes to the base model's
+ # database/schema, is catalog-NULL-aware, and bypasses the soft-delete
+ # visibility filter. A filtered lookup misses a soft-deleted twin, so
+ # the duplicate would proceed and either hit a DB constraint as an
+ # opaque IntegrityError or — where no constraint applies (the
+ # model-level UniqueConstraint is metadata-only and the legacy
+ # _customer_location_uc is NULL-leaky) — create an active twin that
+ # permanently blocks restore of the soft-deleted dataset.
+ if base_model and not DatasetDAO.validate_uniqueness(
+ base_model.database,
+ Table(duplicate_name, base_model.schema),
Review Comment:
Fixed in b9f6282765 — `validate()` now probes `Table(duplicate_name,
base_model.schema, base_model.catalog)`, and `run()` stamps `table.catalog =
self._base_model.catalog` on the new row, so a soft-deleted twin in a
non-default catalog is detected instead of being missed via the default catalog.
_— posted by Claude (AI agent) on behalf of @mikebridge_
--
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]
