hughhhh opened a new pull request, #45848: URL: https://github.com/apache/superset/pull/45848
### SUMMARY Two problems with partition filter mapping, both on master. **Edits silently turned pruning off (SC-130258).** Three edits cleared the mapped column's value transform without asking: - changing the dataset's default datetime column - clearing or changing the partition column - **Move mapping to this column** Once saved, every chart on the dataset stopped pruning partitions. Flip-flopping the column, or clearing it and picking it again, could not bring the transform back. The editor now asks first: **Discard the value transform?** names the column and explains the cost. **Cancel** has focus and leaves every field as it was. Clearing still happens only on confirmation, so transforms never linger on inactive columns (the SC-121834 invariant). Transforms stranded on columns the mapping doesn't mirror are still cleaned up silently, and the explicit **Remove mapping** action doesn't ask. **"Last week" and the editor banner trusted an unchecked verdict (SC-130302).** The mapping's verdict is `evaluable: null` after a deploy, after the 1-day cache expires, or when nobody has previewed the transform. In that state, a new native filter still got the "Last week" default, and the editor still said "Filters on … will automatically apply…". For a broken transform, that meant a full scan while the value list was silently cut to last week. The native filter form also read the dataset through `cachedSupersetGet`, whose cache never expires, so a changed verdict wasn't seen until a page reload. Now: - the default applies only when `evaluable === true` - the form fetches the dataset fresh - the banner shows a new "has not been confirmed" warning until a preview or a stored check confirms the transform - a stored verdict counts only for the mapped and partition columns it was computed for, because the backend keys it on the transform text alone Each eligible new filter still gets the default; that was a deliberate product decision. Verdict identity on the backend (SC-128853) is a separate fix and isn't duplicated here. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Captured against the running app (Postgres dataset, `PARTITION_FILTER_MAPPING` on). Screenshots will be attached: - Changing the default datetime column → confirmation dialog, with Cancel focused - After Cancel: still `event_time`, mapping still confirmed - After Discard: mapping inert on `ingest_time` - Unchecked verdict → "Partition pruning on event_time has not been confirmed" - New native filter: no default with `evaluable: null`; "Last week" once the preview confirmed the transform ### TESTING INSTRUCTIONS 1. Enable `PARTITION_FILTER_MAPPING`. Map a temporal column (the default datetime column) to a partition column with a working, order-preserving transform, and save it through the API so that `evaluable` is `null`. 2. Open the dataset editor → Columns. The banner says pruning has not been confirmed. Click **Check the value transform →**; after a valid preview it says filters will be mirrored. 3. Change **Default datetime column**. The dialog appears. **Cancel** keeps the old column and the transform. Repeat and choose **Discard transform**: the mapping goes inert on the new column. 4. Clearing the partition column shows the same dialog. 5. On a dashboard, add a new Value filter on the dataset. With an unchecked verdict there is no pre-filter. Once a preview has confirmed the transform, the filter starts with "Last week" on the mapped column. Automated: `jest src/components/Datasource src/dashboard/components/nativeFilters/FiltersConfigModal` (54 suites). The new tests fail with either fix reverted. Proved with Jest (54 suites, new tests checked to fail without each fix), `tsc`/oxlint on the changed files, and a Playwright walk of the running app. Reviewed by a Claude review agent in place of the usual Sol review (Sol was unavailable): 2 rounds, approved. ### ADDITIONAL INFORMATION - [x] Has associated issue: Shortcut SC-130258, SC-130302 - [x] Required feature flags: `PARTITION_FILTER_MAPPING` - [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 - [ ] Introduces new feature or API - [ ] Removes existing feature or API Created with Claude Opus 5.5 in T3 Code. 🤖 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]
