rusackas opened a new pull request, #45754:
URL: https://github.com/apache/superset/pull/45754
### SUMMARY
Follow-up to #45431. That PR closed most of `has_extra_cache_key_calls`'s
gaps for
fields `get_sqla_query` renders through the template processor, but two more
surfaced in review there and were deliberately left out rather than widening
that PR (both flagged by @gabotorresruiz):
- The gate checked `granularity`'s truthiness (`if granularity :=
query_obj.get("granularity")`)
rather than whether it was provided at all. `get_sqla_query` falls back
from
`granularity` to `main_dttm_col` whenever `granularity` is not one of the
dataset's
temporal columns, and an empty string satisfies that the same as any other
non-matching name. The truthiness check meant `granularity=""` skipped the
fallback entirely and missed a macro in `main_dttm_col`'s expression.
- When a dataset has `always_filter_main_dttm` set and `main_dttm_col`
differs from
the column `granularity` resolved to, `get_sqla_query` adds a second,
independent
time filter on `main_dttm_col` (`helpers.py`, the block right after the
primary
time filter), rendering that column's own expression regardless of which
column
the query actually named. The gate never accounted for that second filter,
so a
macro there could leak between two principals the same way the fields in
#45431
did.
Both fixes follow the same approach as #45431: resolve the field the way
`get_sqla_query` actually resolves it, and add it to
`templatable_statements` so a
macro there triggers the extra-cache-key extraction.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A, backend-only.
### TESTING INSTRUCTIONS
```
pytest tests/integration_tests/sqla_models_tests.py -k extra_cache_keys
```
`test_extra_cache_keys_for_granularity_fallback_and_always_filter_main_dttm`
covers
the gate (`has_extra_cache_key_calls`) across both fixed conditions plus
negative
controls (no `granularity` at all, the flag unset, `granularity` already
matching a
temporal column).
`test_extra_cache_keys_differ_for_granularity_fallback_and_always_filter_main_dttm`
asserts `get_extra_cache_keys` for the two positive cases, since the gate
firing and
the key actually differing are two separate claims.
Reverting either fix on its own fails exactly the cases tied to it (checked
locally
via `git stash` on `models.py`).
### 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]
