aminghadersohi commented on code in PR #40129:
URL: https://github.com/apache/superset/pull/40129#discussion_r3342337449


##########
UPDATING.md:
##########
@@ -30,6 +30,18 @@ Importing a dataset now validates the `catalog` field 
against the target databas
 
 If you relied on importing datasets with a non-default catalog, enable "Allow 
changing catalogs" on the target connection, or set the dataset's catalog to 
the connection's default before importing.
 
+### Soft delete and restore for charts
+
+`DELETE /api/v1/chart/<id>` no longer hard-deletes the chart. The row is 
marked with a `deleted_at` timestamp and hidden from all list, detail, and 
lookup endpoints. Charts in this state are excluded from default queries and 
from relationship loads (e.g. `dashboard.slices`).
+
+**New endpoint** — `POST /api/v1/chart/<uuid>/restore` clears `deleted_at` and 
returns the chart to active state. Requires `can_write on Chart` and ownership 
of the row (or admin). Soft-deleted charts can also be surfaced in the list 
endpoint via the new `chart_deleted_state` rison filter: `include` returns both 
live and soft-deleted rows, `only` returns just the soft-deleted ones. Any 
other value is ignored.
+
+**Permissions migration:** existing role grants of `can_write on Chart` cover 
the new restore endpoint automatically; no role migration is required.
+
+**Schema migration:** the migration adds a nullable `deleted_at` column and an 
index on it (`ix_slices_deleted_at`) to the `slices` table. The column add is 
instant; the index build runs inline (no `CONCURRENTLY`) and may briefly block 
reads on the `slices` table for the duration of the build on large Postgres 
deployments. MySQL InnoDB builds the index online (no blocking).

Review Comment:
   **HIGH — factual error: PostgreSQL `CREATE INDEX` blocks writes, not reads**
   
   > *may briefly block reads on the `slices` table for the duration of the 
build on large Postgres deployments*
   
   `CREATE INDEX` (non-CONCURRENT) acquires a `ShareLock`, which conflicts with 
`RowExclusiveLock` (INSERT/UPDATE/DELETE) but **not** with `AccessShareLock` 
(SELECT). Reads continue uninterrupted during the build; it is writes that are 
queued.
   
   This also contradicts the PR description, which says *"non-blocking on 
Postgres."*
   
   Suggested correction:
   > *the index build runs inline (no `CONCURRENTLY`) and may briefly block 
writes on the `slices` table (INSERT/UPDATE/DELETE are queued while the index 
builds; reads are unaffected) on large Postgres deployments. MySQL InnoDB 
builds the index online (no blocking).*



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

Reply via email to