mikebridge opened a new pull request, #45294:
URL: https://github.com/apache/superset/pull/45294
### SUMMARY
**TL;DR:** A semantic-view chart whose own filter names a column the view no
longer has now returns a clear error. Before, it silently returned unfiltered
(wider) results.
**Background.** Semantic-view charts filter on the view's dimensions. A
chart can keep a filter on a column the view later drops or renames. Dashboard
native filters and cross-filters can also target columns from other datasets.
**Problem.** When a chart's *own* filter named a column the view does not
have, the filter was skipped and only listed as rejected. The chart rendered
successfully, with more rows than its saved definition allows. This is easy to
miss in a table and wrong in a total, and it carries into scheduled reports and
alerts.
**What this changes.**
- A chart-defined filter (no `isExtra`) on a column that is neither a
dimension nor a metric of the semantic view returns a 400. The error names the
column and says to select a dimension or remove the filter. Chart-defined
metric filters were already refused this way.
- Dashboard native filters and cross-filters (`isExtra`) on such columns
keep today's behavior: the chart renders, and the filter is reported as
**Incompatible**.
- A "No filter" time range is not refused. It constrains nothing, and
Explore can store one on a dashboard's time column when a chart is saved from a
dashboard.
This intentionally differs from SQL datasets, which skip and report unknown
filter columns, and from the "matching SQL datasets" note added in #45105. A
semantic chart's filter on a removed dimension is a stale definition, not a
cross-dataset dashboard filter, so failing closed is preferable to a silently
widened answer.
**Technical details**
- `superset/semantic_layers/mapper.py`:
- `validate_filter_columns` adds the missing-column refusal next to the
existing metric refusal, and skips no-op time ranges.
- It already runs in `QueryObject.validate()` before result-cache access,
and in the mapper's validate/map entry points. A cached widened result
therefore cannot bypass it.
- Cache keys, `result_cache_version` and `selection_identity_version` are
unchanged.
- Both refusal messages share one shape. The `messages.pot` template is
updated.
- `UPDATING.md` and the dashboard docs describe the change. They also note:
- Scheduled reports, alerts and thumbnails for affected charts fail until
the chart is edited.
- Open dashboards should be reloaded after deploying, because older
browser bundles do not mark dashboard filters with `isExtra`.
- A dashboard filter carried into a new chart (drill-by "Edit chart")
becomes a chart filter.
Tracked internally as SC-126719.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI change.
- **Before:** a chart-data request for a semantic chart whose own filter
names a removed column returned 200, with that filter in
`rejected_filter_columns` and unfiltered rows.
- **After:** it returns 400, naming the column.
### TESTING INSTRUCTIONS
1. Enable `SEMANTIC_LAYERS`. Create a semantic-view chart with a filter on a
dimension, then remove or rename that dimension in the provider (or send the
request directly). `POST /api/v1/chart/data` with `{"col": "missing_column",
"op": "IN", "val": ["US"]}` returns 400 naming `missing_column`, and the
provider is not queried.
2. Send the same filter with `"isExtra": true`. The chart renders and lists
the column under `rejected_filter_columns`.
3. Send `{"col": "ds", "op": "TEMPORAL_RANGE", "val": "No filter"}` on a
column the view lacks. The chart renders.
4. Run `pytest tests/unit_tests/semantic_layers/metric_filter_test.py`.
Automated verification:
- The new refusal cases failed before the fix (5 failed) and pass after it.
They cover the HTTP route, refusal before the result cache, and both direct
mapper entry points. The no-op time-range cases were added red-first as well.
- `tests/unit_tests/semantic_layers/`: 1335 passed.
- Changed-file pre-commit hooks passed, including MyPy and the `.pot` drift
check.
- Full local unit suite: the only failures are pre-existing
local-environment ones that also fail on `master`:
- 8 MCP dateutil/DST timezone tests;
- one `longdouble` test;
- one browser numeric-contract test;
- one Babel fixture test.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `SEMANTIC_LAYERS` (existing)
- [ ] 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]
