tritsystem opened a new issue, #43918:
URL: https://github.com/apache/superset/issues/43918

   ### Summary
   
   Using the "Refresh columns" / sync-columns-from-source action on an existing 
dataset does not bump `SqlaTable.changed_on`, so the chart-data cache key 
(`query_context_processor.query_cache_key`) does not change either — meaning a 
previously cached chart/dashboard query result computed against the OLD column 
type/expression can keep being served after the dataset's schema is refreshed, 
exactly when freshness matters most (the underlying table changed).
   
   ### Root cause
   
   - `superset/common/query_context_processor.py` `query_cache_key()` 
(~L405-428) builds the cache key from `extra_cache_keys` plus 
`changed_on=datasource.changed_on`. For a physical `SqlaTable`, 
`get_extra_cache_keys()` (`superset/connectors/sqla/models.py` ~L2497, base 
impl ~L814-825) adds nothing beyond RLS predicates for virtual datasets — 
`changed_on` is the only signal tying the cache key to column/metric 
definitions.
   - The manual-edit path is patched for this: `superset/daos/dataset.py` 
`DatasetDAO.update()` (~L410-437) explicitly force-sets 
`attributes["changed_on"] = datetime.now()` whenever the payload contains 
`"columns"` or `"metrics"` — i.e. `PUT /api/v1/dataset/<id>` always bumps 
`changed_on`.
   - The refresh path is **not** patched: 
`superset/commands/dataset/refresh.py` `RefreshDatasetCommand.run()` (L53) 
calls `self._model.fetch_metadata()` directly, bypassing `DatasetDAO.update()` 
entirely.
   - `SqlaTable.fetch_metadata()` (`superset/connectors/sqla/models.py` 
L2269-2369) mutates existing `TableColumn` rows in place (`new_column.type = 
...`, `new_column.expression = ...`, L2341-2342), reassigns the relationship 
wholesale (`self.columns = columns`, L2359), and calls `add_missing_metrics()` 
(L416-422, which only *appends brand-new metric names* — an existing metric's 
SQL expression is never touched). It never sets `self.changed_on` itself, and 
the one `SqlaTable`-owned field it can touch, `main_dttm_col` (L2361-2362), is 
only set `if not self.main_dttm_col` — a no-op for any dataset that already has 
a temporal column chosen (the common case for any dataset that's already in 
use).
   - `changed_on` has `onupdate=datetime.now` (`superset/models/helpers.py` 
`AuditMixinNullable`, ~L1261). SQLAlchemy fires a column's `onupdate` only when 
an `UPDATE` is emitted for that specific row. Since `TableColumn.table_id` is 
the FK (child owns it), reassigning `SqlaTable.columns` produces `UPDATE`s on 
the child `table_columns` rows, not the parent `tables` row. If no column owned 
by `SqlaTable` itself changes, no `UPDATE` is issued for the `tables` row, so 
`changed_on`'s `onupdate` never fires.
   
   Net effect: refreshing a dataset's columns from the source DB can leave 
`changed_on` untouched, so `query_cache_key()` returns an unchanged key, and 
Superset keeps serving the previously cached result computed under the old 
column definition.
   
   ### What was executed vs. traced
   
   - **Traced by reading source** (citations above), confirmed present and 
unchanged on `master` via `gh api` blob-hash comparison against a fresh `git 
clone --depth 1 apache/superset` (hashes matched exactly for 
`superset/commands/dataset/refresh.py`, `superset/daos/dataset.py`, and 
`superset/common/query_context_processor.py`).
   - **Actually executed**: a standalone SQLAlchemy repro (SQLAlchemy 2.0.52, 
matching Superset's `pyproject.toml` pin) with a minimal model shaped exactly 
like `SqlaTable`/`TableColumn` (same relationship direction, same 
`onupdate=datetime.now` pattern). It confirms mutating a child row's own column 
+ reassigning the parent's relationship collection, with no change to any 
column the parent itself owns, does **not** fire the parent's `onupdate` — 
while a genuine parent-column change (control case) does.
   
   ```
   changed_on after initial create: 2026-09-04 20:00:01.724460
   
   Simulating a schema change detected by fetch_metadata(): column 'revenue' 
type VARCHAR -> INTEGER
   changed_on after simulated fetch_metadata() column-type change: 2026-09-04 
20:00:01.724460
   
   *** CONFIRMED: reassigning the relationship collection + mutating a child 
row's own column, with no change to any of the PARENT's own mapped columns, did 
NOT bump the parent's changed_on (onupdate did not fire). ***
   
   control: changed_on after renaming table (parent column changed): 2026-09-04 
20:00:03.934878
   control confirms onupdate mechanism works correctly when a parent column IS 
touched -- so the refresh-path gap above is real and specific to the 
relationship-only mutation, not a broken test setup.
   ```
   
   - **Not executed**: a full live Superset app end-to-end repro (real Flask 
app + DB, calling the actual HTTP refresh endpoint and then the chart-data 
endpoint to observe a stale response) was out of scope for the time budget of 
this pass. The isolated mechanism repro plus the traced code path is the 
evidence offered here; I'm filing this as a traceable, mechanism-verified 
report rather than a full live capture, and am happy to help verify further if 
a maintainer wants a live-app confirmation.
   
   ### Suggested fix
   
   `RefreshDatasetCommand.run()` (or `SqlaTable.fetch_metadata()` itself) could 
explicitly set `self._model.changed_on = datetime.now()` — mirroring 
`DatasetDAO.update()`'s existing `force_update` handling — whenever 
`fetch_metadata()` reports any `added`/`removed`/`modified` columns, so the 
chart-data cache key is invalidated on the same schema-drift signal already 
surfaced to the user in the refresh response (`MetadataResult.modified`).
   
   ### Prior-art check
   
   Searched `gh search issues`/`gh search prs` on `apache/superset` for 
variants of "refresh dataset stale cache", "changed_on cache", "sync columns 
cache", "cache not invalidated", "fetch_metadata changed_on", 
"RefreshDatasetCommand", etc. — sanity query (`"dashboard"`) returns normal 
results, but none of the targeted queries surfaced an existing report of this 
specific gap. PR #42463 touches the same `fetch_metadata()`/refresh path but 
for an unrelated Jinja-SQL parse-failure bug.
   
   ---
   Found via a property-based audit (copy/clone independence, 
cache-invalidation consistency) run across several widely-used Python 
libraries; the SQLAlchemy-mechanism repro was written and independently re-run 
before filing.
   


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