You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/10/08 23:26:37 UTC
[PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
mikebridge opened a new pull request, #45109:
URL: https://github.com/apache/superset/pull/45109
### SUMMARY
#44370 disabled column-value suggestions on every versioned semantic view. A saved selection's display title can exactly match a *different* member's current stable ID, so the server can't tell from the column name alone whether a saved filter refers to a current member. This PR restores suggestions for selections whose provenance can be checked.
**How the column-values endpoint changes (`GET /api/v1/datasource/<type>/<id>/column/<col>/values/`):**
- It accepts an optional `semantic_selection_version` query parameter.
- On a versioned semantic view, a missing or stale version gets the existing `200` response, with `result: []` and `suggestions_status: "unavailable_versioned_view"`. That response is returned before any cache read or provider call, as it is today.
- A matching version enables the normal retrieval and search path.
- The version is part of the values cache key, but only for versioned views, so cached values from an older identity version are never served after a version bump. The cache key for tables and unversioned views doesn't change.
**How the frontend changes:**
- The value comes from the chart's own `form_data.semantic_selection_version`. It is never taken from the datasource metadata that was just fetched.
- Explore stamps that marker only when it resets semantic selections, which rebuilds form data and clears filters. A chart that carries the current version therefore had its fields picked after reselection.
- Both the Dnd filter control and the legacy adhoc filter control pass the marker through to the simple-tab value picker. The path is `DndFilterSelect` / `AdhocFilterControl` → filter option → popover trigger → edit popover → simple tab.
- Matrixify sends it too.
- Dashboard extra filters don't inherit the chart's marker.
**Compatibility:**
- Tables and unversioned semantic views behave as before.
- Older selections without a current marker fall back to manual entry, which is the current behavior. Users can reset and reselect fields to get suggestions back.
- Rolling deploys are safe in both directions: an old frontend sends no version, and an old backend ignores it. Either way the result is manual entry.
- No migration and no change to persisted shape.
`UPDATING.md`, the semantic member selections docs and the OpenAPI spec are updated.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not captured. Before: the filter value picker and Matrixify value list on a versioned semantic view always showed "enter values manually". After: they load suggestions once fields have been explicitly reselected; selections that haven't been reselected still show the manual-entry guidance.
### TESTING INSTRUCTIONS
Automated:
```bash
pytest tests/unit_tests/semantic_layers/values_endpoint_test.py \
tests/unit_tests/semantic_layers/selection_identity_test.py
cd superset-frontend && npm run test -- \
src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx \
src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx \
src/explore/components/controls/MatrixifyDimensionControl.test.tsx
```
The backend tests include a collision fixture: a legacy title that equals another member's current ID never reaches the cache or provider without a matching version. They also cover cache partitioning by version. The frontend tests check that each control sends the chart's marker, and only that marker, for missing, stale and current versions, using the real control mapper. They also cover a late failing response racing a newer success.
Manual, with a semantic layer whose views report a selection identity version:
1. Open an existing chart on a versioned semantic view that was saved before its fields were reselected. Add or edit a filter. The value picker offers manual entry only.
2. Click **Start field selection**, reselect the fields, and edit the filter again. Suggested values load, and search works.
3. In Matrixify, **Members** and alphabetically sorted **All** modes load values after reselection.
4. Table-backed charts are unchanged.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] 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
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
Follows #44370. The new API surface is an optional query parameter on an existing endpoint.
🤖 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6071052543
@aminghadersohi @rebenitez1802 could you take a look when you have a chance? This restores filter-value suggestions on versioned semantic views, gated on the chart's own selection version. It's the follow-up to #44370.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6071313462
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45109?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `94.44444%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 66.19%. Comparing base ([`34b3d65`](https://app.codecov.io/gh/apache/superset/commit/34b3d65cc208600e0152017ba0db705a3d2b57bd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1d80ef6`](https://app.codecov.io/gh/apache/superset/commit/1d80ef6da948ea093b504ded2e0909b54a53791c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 9 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/45109?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/datasource/api.py](https://app.codecov.io/gh/apache/superset/pull/45109?src=pr&el=tree&filepath=superset%2Fdatasource%2Fapi.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGF0YXNvdXJjZS9hcGkucHk=) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/45109?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (34b3d65) and HEAD (1d80ef6). Click for more details.
>
> <details><summary>HEAD has 4 uploads less than BASE</summary>
>
>| Flag | BASE (34b3d65) | HEAD (1d80ef6) |
>|------|------|------|
>|python|7|3|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #45109 +/- ##
===========================================
- Coverage 82.64% 66.19% -16.46%
===========================================
Files 3007 3009 +2
Lines 187318 187978 +660
Branches 43399 43518 +119
===========================================
- Hits 154811 124426 -30385
- Misses 29742 60803 +31061
+ Partials 2765 2749 -16
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.16% <0.00%> (-0.02%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.39% <100.00%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.01% <0.00%> (-0.04%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.07% <50.00%> (-31.39%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.94% <50.00%> (+0.09%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/45109/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45109?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #45109:
URL: https://github.com/apache/superset/pull/45109
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6071072583
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 1d80ef6da948ea093b504ded2e0909b54a53791c |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ac826ad3cae0d0007115a1a |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45109--superset-docs-preview.netlify.app](https://deploy-preview-45109--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctLCoAwDADRu2RtofFTxRt4jJgGFbQtpgoi3t3uZvHmhXTGI2UYASqYTwq8llY2WGPbOaNyUMgbm5v2S4xeyyKatxi0eC9pj8_ky-GIh9qRb5jEemttj9gREnw_) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-45109--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6074433090
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge; the changes pass selection provenance through filter and Matrixify value requests and gate versioned-view suggestions before cache access.
- Added tests cover missing, stale, and matching versions, cache separation, and late filter responses; no unresolved concrete concern is apparent.
**Assessed commit:** `1d80ef6da948`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(semantic-layer): restore value suggestions for semantic view filters [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6075418311
Thanks @aminghadersohi, this was a careful read. Changes are in 77d82cd5ca.
1. **The marker is an assertion, not proof.** Agreed. UPDATING already calls it "a compatibility contract, not an authorization credential". I've said the same in the code comment at the gate and in the user docs: anyone can copy the version from datasource metadata, it only prevents stale-title collisions, and authorization is checked before it.
2. **Chart-level marker vs per-filter provenance.** Added a docs line: the version is recorded per chart, so a filter column added by hand-editing saved form data inherits it. Suggestions are a usability aid for compatible selections, not a per-filter provenance guarantee.
3. **Remount drops typed text.** Intended. The key changes only on reselection or a datasource change, and clearing the previous generation's options matters more there than keeping half-typed text.
4. **Can loading stick?** In one narrow case, yes. An in-flight load aborted by a change that keeps the values select visible but takes an early return (switching to a datasource with `filter_select: false`) skipped its own reset. I fixed it by resetting loading at the start of each effect run. The new test fails without the fix.
5. **Test coverage.**
- *Cache key:* `test_unversioned_suggestion_cache_material_is_unchanged` asserts that an unversioned view's key material keeps exactly the previous five fields, even when a marker is sent. Together with the existing legacy-bytes test of `_column_values_cache_key`, that pins the key end to end. Tables take the same path with no selection version.
- *`isExtra`:* already covered. The `test.each` case `['cube-member-id-v1', true]` in `AdhocFilterEditPopoverSimpleTabContent.test.tsx` expects no marker for dashboard extra filters.
6. **Silent `undefined`.** Agreed it's silent. It degrades to manual entry, which is the safe direction, so I've left it as is.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org