hughhhh opened a new pull request, #45992: URL: https://github.com/apache/superset/pull/45992
### SUMMARY Explore and the dataset editor can say a filter is mirrored onto the partition column when the query carries no such filter (SC-128853). The "transform works" verdict was cached per connection and transform text, so one column's answer stood in for every other column on that connection. A type-mismatch decline was never recorded at all, so it stayed `true`. The verdict now belongs to one dataset, mapped column and partition column, and `true` means a query actually carried the predicate. - **Key:** adds the dataset id, both column names, and their types and date formats. A column sync that retypes a column retires its verdict. - **Where it is recorded:** in `build_mirrored_predicates`, not the probe. The probe cache is shared across datasets, so a cache hit never reached the old recording. With a per-dataset key, that dataset would have stayed unsettled for the whole timeout. Only an engine refusal is still recorded next to the probe, because a refusal isn't cached and `None` can't be told apart from an input that was declined without asking the engine. - **Type mismatches:** recorded per column. This was removed earlier because the key had no partition column: `CAST(:value AS text)` on a text key in dataset A was reported `evaluable: false` after someone previewed it on a numeric key in dataset B. That can't recur with this key, and it is tested in both directions. - **Cold cache:** `mirrorable_operators` is `[]` until a verdict confirms the mapping. The glyph and the time range control both read that field, so nothing is promised before a probe. `evaluable` stays `null`, not `false`, so the editor still shows "not confirmed" rather than "broken". This also closes a gap: the time range control never read `evaluable`, so it kept promising after a refusal. Probing on read would also fix the cold case, but it would add a warehouse round trip to every Explore and dashboard load, so I didn't do that. #### Behaviour changes to check - **A working mapping shows no icon until a chart or preview on it has carried a mirror**, and the page has been reloaded, since the payload isn't refetched. This is a deliberate trade: the UI may under-promise briefly rather than over-promise. - **`CACHE_CONFIG = NullCache` (the default):** the icon and time range indicator never appear, because nothing can be confirmed. The docs previously described this as "stays optimistic" and are updated. Pruning is unaffected. - **On deploy:** every verdict goes cold, so these indicators disappear until each mapping's next mirrored chart. The verdict key has a new version (`v3`) and a new digest, so an old entry that may be another column's answer is never read. The version bump is redundant given the digest change; it is kept for the file's convention. `PROBE_CACHE_VERSION` stays `v2` on purpose: probe answers mean the same as before, the first chart settles the verdict from them without a round trip, and bumping it would empty the canary answers behind `safety_refusal`. #### What #45848 changes for the API No new fields needed. That PR already matches a stored verdict on `evaluated_transform`, `mapped_column` and `partition_column`, and treats only `true` as confirmed. It relies on `null` staying distinct from `false`, which is preserved. #### Not in this PR: case 4 icon gaps These are per-value gates in the frontend's `isMirroredFilter` and `timeRangePartitionMapping`. The verdict describes a mapping, so it can't express them, and recording a declined value as `false` would bring back the withdrawal bug above. | Chip | Server | Client | |---|---|---| | `== '20261021T120000'` on DATE | declines | shows; `ISO_DATETIME` only matches extended format | | `>= '2026-10-21 10:00:00 PM'` on DATE | declines | shows; `ISO_DATE_PREFIX` passes | | `>= '10/21/2026 12:00'` on TIMESTAMP (Hive) | transform answers `NULL`, no mirror | shows; the answer depends on the engine, so no client pattern can know | | Legacy Time Range with a default-filled Time Column | n/a | missing; not verified against the code path beyond reading it | The third row needs the query result itself to say whether it mirrored, or a value-level check. These need a frontend follow-up. That follow-up should also update the `mirrorsAnything` comment in `PartitionPruningIndicator`, which still says `null` is no reason to go quiet; the server now sends no operators in that state. Also left alone: - Save is still allowed for a transform the preview says won't mirror. - Shape declines, inverted bounds and answers that can't be written as literals record nothing, because they depend on the input. They no longer record `true` either. - `CAST(:value AS date)` onto a DATE key on Hive doesn't mirror, because pyhive returns dates as strings and the existing type gate declines them. That gate isn't changed here; the verdict just reports it honestly now. It looks worth its own ticket. #### Overlap #45851 adds `record_refusal` along the same `evaluate_transform` to `_run_probe` path, so expect a small rebase conflict there. The two should compose: this PR already restores `true` on a probe cache hit after a refused text form. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not captured: there is no frontend change and I did not run this in a browser. Results from the real-engine walk, where "glyph" is the client's gates applied to the API summary: | Case | Engine | Before | After | |---|---|---|---| | 1. `unix_timestamp(trunc(from_unixtime(:value), 'HH'))`, first load on a cold cache | Hive 4.0.1 | glyph on, no mirror in SQL | glyph off, `evaluable: null`; `false` after a chart | | 2. Integer transform on a text key, each chart declined | Postgres 17 | `evaluable: true`, glyph on | `evaluable: false`, glyph off | | 2. Same text on a BIGINT key, probe cache hit | Postgres 17 | `true` | `true`, mirror in SQL, text-key dataset still `false` | | 3. `:value - :value % 3600` previewed on epoch, read on TIMESTAMP | Hive 4.0.1 | TIMESTAMP `true`, glyph on, no mirror | TIMESTAMP `null`, then `false` after a chart; epoch stays `true` | | 3. Reverse order | Hive 4.0.1 | untouched epoch dataset flips to `false` | epoch stays `null`, then `true`; TIMESTAMP stays `false` | | Two datasets, `CAST` to text/string, both orders | Hive 4.0.1, Postgres 17 | not separated: Postgres reports B `true` while it declines | A `true`, B `false`, whichever goes first | ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/connectors/sqla/partition_verdict_identity_test.py pytest tests/unit_tests/connectors/sqla/ tests/unit_tests/models/partition_mirroring_test.py tests/unit_tests/datasets/ ``` The regression tests read the verdict through `partition_filter_mapping_summary`, the same way the UI does. 14 of the first 18 failed on unmodified master on their assertions; the other 4 were guards that already passed. 3 more were added while implementing and were not watched failing on master. Mutating each part of the fix (key fields, each recording site, the summary gate) turned tests red, except the version bump noted above. To try it manually on Hive with `PARTITION_FILTER_MAPPING` on and a real `CACHE_CONFIG`: 1. Map an epoch column onto a BIGINT partition column with `unix_timestamp(trunc(from_unixtime(:value), 'HH'))` and flush the cache. Open Explore and add a filter: no icon. 2. On two datasets on one connection, set `:value - :value % 3600` on an epoch column and on a TIMESTAMP column. Preview the epoch one, then GET the TIMESTAMP dataset: `evaluable` is `null` and `mirrorable_operators` is empty. **Tested:** Apache Hive 4.0.1 (HiveServer2 in Docker, via pyhive) and Postgres 17. Probes ran on those engines. Chart SQL was compiled and inspected rather than executed, and the metadata DB was SQLite. **Not tested:** Impala, Trino, Oracle, any other engine, and a browser. No frontend test was run because no frontend code changed. **Full backend unit suite:** 36,492 passed, 13 failed. The same 13 fail on clean master here and are unrelated to this change (date parsing, jinja time filters, longdouble handling, MCP time ranges). ruff, pylint, mypy and pre-commit pass on this branch. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [x] Required feature flags: `PARTITION_FILTER_MAPPING` - [x] Changes UI (through the API summary; no frontend code changes) - [ ] 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 Claude 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]
