mikebridge opened a new pull request, #44089:
URL: https://github.com/apache/superset/pull/44089
### SUMMARY
`_dashboard_slice_uuids_at_tx` — the change-record diff's "which charts were
on
this dashboard at transaction *tx*" read — used the raw
`dashboard_slices_version`
validity predicate (`end_transaction_id IS NULL OR > tx` + `operation_type
!= DELETE`).
SQLAlchemy-Continuum **never closes `end_transaction_id` on an M2M
association
shadow**, so a chart attached at tx 1 and removed at tx 5 still read as a
member
at tx 10 — over-reporting membership in the dashboard's change-record diff
(and
emitting phantom slice records for charts that were already gone).
This is the **third consumer** of that broken pattern; the version-restore
membership read (`restore.py`) and the impact rollup (`impact.py`) were
fixed the
same way in #44010 (sc-119907), and Amin flagged this change-record diff
site as
the known remaining one.
The fix resolves membership through `charts_attached_to_dashboard`, which
pairs
the association shadow's INSERT/DELETE rows into half-open `[attach, detach)`
windows (via `attachment_windows`), and keeps only charts whose window
contains
*tx*. The `slices_version` (content) shadow **does** close
`end_transaction_id`
correctly, so its validity predicate is kept for uuid resolution and the
"not yet
versioned at tx" exclusion (matching Continuum's M2M `Reverter`).
Threading + performance details (from the local 4-lens review):
- The committing `session` is threaded into `charts_attached_to_dashboard`
(new
optional param, defaulting to `db.session`). This read runs during commit
finalization, when the current transaction's association-shadow rows are
flushed-but-not-committed and visible only on the committing connection —
reading via the global `db.session` would miss them whenever the committing
session isn't the Flask-scoped one. Read/restore-path callers keep the
default.
- The attachment windows are computed once per dashboard and reused for the
pre/post-tx membership (they're tx-independent), instead of re-scanning the
association history twice.
- The `slices_version` id IN is chunked via `chunked_ids`, mirroring the
sibling
impact rollup, so a pathologically wide dashboard can't exceed the SQLite
bind
floor.
Refs: sc-120007. Follow-up to #44010 / sc-119907.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI change — backend versioning correctness. Behaviorally: a dashboard's
version-history/change-record diff no longer lists a chart that had already
been
removed before the transaction being described.
### TESTING INSTRUCTIONS
Unit: `pytest tests/unit_tests/versioning/test_shadow_queries.py` — a chart
detached before the target tx is excluded from resolved membership while a
still-attached (or re-attached) chart is included; empty-membership
short-circuit
and NULL-uuid filtering covered; includes a reverted-fix control. The read
path
is also exercised end-to-end by
`tests/integration_tests/versioning/change_records_tests.py`.
Manual: attach a chart to a dashboard, save; remove it, save; edit the
dashboard
again, save. The change record for the last save must not report the removed
chart as a member.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
