hughhhh opened a new pull request, #43870: URL: https://github.com/apache/superset/pull/43870
### SUMMARY Sixth and last of the partition filter mapping stack, implementing **wireframe 1d** — the piece that tells a chart author *why* their query got faster. Stacked on `hughhhh/pfm-5-editor-ui` (#43891). Review the [compare against pfm-5](https://github.com/apache/superset/compare/hughhhh/pfm-5-editor-ui...hughhhh/pfm-6-explore-indicator) rather than the diff against master. A small glyph appears on any filter whose column the dataset mirrors onto its partition column, with the tooltip *"This filter is also applied to a partition column for faster queries. See 'View query' for the generated SQL."* Chart authors configure nothing and ideally never learn the word "partition"; per the PRD the indicator exists only to explain the speed-up and point at the SQL. **Where it renders.** Explore models the time range as a `TEMPORAL_RANGE` adhoc filter, so the time range and ordinary filters are the same chip — one indicator covers both cases from 1d. The path that actually renders is `DndAdhocFilterOption` → `OptionWrapper` → `Option`; `OptionControlLabel` and the legacy `AdhocFilterOption` are wired too, so the glyph does not silently vanish on whichever surface uses them. The standalone `time_range` control gets the mapping through a new `mapStateToProps`, for viz types that still have one. **Naming.** `partitionColumn` already exists on these components as the unrelated Presto `latest_partition` feature, so this is `partitionMapping` throughout to avoid two meanings of the same word one prop apart. **Also fixes a bug this made visible.** `partition_filter_mapping_summary` was not gated on the feature flag, so with `PARTITION_FILTER_MAPPING` off the payload still reported `active: true`. Nothing is mirrored in that state — `resolve_partition_mapping` returns `None` — so the indicator was promising a predicate the query never carried. The summary now gates the same way the query path does. The bug has been in the stack since pfm-1; it was invisible until something rendered from it, which is exactly what this PR does. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF _Attaching separately (see the screenshot list at the end of Testing Instructions). The clearest pair is `02-adhoc-operator-matrix.png` — one chart carrying eight filters, glyph on exactly the two that mirror — and `03-tooltip.png`._ ### TESTING INSTRUCTIONS Automated: 821 backend and 837 frontend tests pass; `tsc`, `ruff`, `ruff format` and `oxlint` clean. Manual validation was re-run end to end against the current head of this branch, in the dev Docker stack (Flask + Celery + Postgres + Redis) built from this worktree, with `PARTITION_FILTER_MAPPING: True`. Fixture is a physical table `public.pfm_events` on the examples database: ```sql event_time TIMESTAMP, ingested_at TIMESTAMP, dt_epoch BIGINT, country VARCHAR, country_key VARCHAR, revenue DOUBLE PRECISION ``` #### 1. Non-monotonic string mapping — the operator matrix Dataset editor → **Columns** → Partition column `country_key`, mapped column `country`, value transform `lower(:value)`, **Transform preserves ordering** unchecked. Payload: ```json {"active": true, "is_monotonic": false, "mapped_column": "country", "mirrorable_operators": ["==", "IN"], "partition_column": "country_key"} ``` One table chart carrying eight filters at once, so every branch of `isMirroredFilter` is visible in a single screenshot: | Filter chip | Glyph | Why | | --- | :---: | --- | | `country = 'US'` | ✅ | `EQUALS` mirrors regardless of monotonicity | | `country IN ('US', 'CA')` | ✅ | `IN` mirrors regardless of monotonicity | | `country <> 'MX'` | — | `NOT_EQUALS` is not mirrorable | | `country > 'CA'` | — | range operators need a monotonic transform | | `revenue > 10` | — | not the mapped column | | `country_key = 'us'` | — | the partition column itself | | `country = 'US'` *(Custom SQL)* | — | `expressionType: 'SQL'` lands in `extras.where` | | `country IN ('US', NULL)` | — | a NULL widens the predicate | | `event_time (No filter)` | — | no usable comparator | `document.querySelectorAll('[data-test="partition-pruning-indicator"]').length` is exactly **2**. Note rows 1 and 7 render the *identical* label `country = 'US'` — one simple, one Custom SQL — and the glyph is the only thing that tells them apart. Hovering the glyph gives, verbatim: > This filter is also applied to a partition column for faster queries. See "View query" for the generated SQL. **View query** on that chart: ```sql WHERE country = 'US' AND country IN ('US', 'CA') AND country <> 'MX' AND country > 'CA' AND revenue > 10 AND country_key = 'us' AND (country IS NULL OR country IN ('US')) AND country_key = 'us' -- mirrored from country = 'US' AND country_key IN ('us', 'ca') -- mirrored from country IN ('US', 'CA') AND ((country = 'US')) ``` Two mirrored predicates for two glyphs. The `IN ('US', NULL)` chip is the interesting one: the backend widened it to `country IS NULL OR country IN ('US')` and emitted no mirror, which is exactly why the frontend withholds the glyph — the two sides agree. #### 2. Monotonic temporal mapping — both surfaces Partition column `dt_epoch`, mapped column `event_time`, transform `extract(epoch from cast(:value as timestamp))`, **Transform preserves ordering** checked. The editor's live preview evaluates it against Postgres and reports `event_time >= '2026-01-15 00:00:00'` → `dt_epoch >= 1768435200.000000`. Payload: ```json {"active": true, "is_monotonic": true, "mapped_column": "event_time", "mirrorable_operators": ["<", "<=", "==", ">", ">=", "IN", "TEMPORAL_RANGE"], "partition_column": "dt_epoch"} ``` **Adhoc filter chip.** `2026-01-01 ≤ event_time < 2026-02-01` carries the glyph; View query shows the mirror: ```sql AND dt_epoch >= 1767225600.000000 AND dt_epoch < 1769904000.000000 ``` Setting the same chip to `No filter` removes the glyph. **Standalone Time Range control.** Checked on `cal_heatmap`, whose control panel is `[['granularity_sqla'], ['time_range']]`: | Time Column | Time Range | Glyph | | --- | --- | :---: | | `event_time` (mapped) | `2026-01-01 : 2026-02-01` | ✅ | | `ingested_at` (not mapped) | `2026-01-01 : 2026-02-01` | — | | `event_time` (mapped) | `No filter` | — | #### 3. Feature flag off Set `PARTITION_FILTER_MAPPING: False`, restart, reload the same chart: the Explore payload serializes `partition_filter_mapping` as `null`, no glyph renders on the chip that had one, and View query carries no `dt_epoch` predicate — the summary and the query path stay in lockstep, which is the bug fixed in this PR. Restored to `True` and re-confirmed the glyph returns. #### Known limitation found while validating `partition_filter_mapping_summary` computes `active` from cheap checks only — the columns resolve, the transform is non-empty, contains `:value` and no Jinja — and deliberately does not parse or evaluate the transform, because that is too expensive for a payload built on every Explore load. So a transform that *saves* but that the warehouse then rejects still reports `active: true`, and the glyph promises a predicate the query does not carry. This is not hypothetical: the obvious Postgres transform `extract(epoch from :value)` fails at query time with ``` function pg_catalog.extract(unknown, unknown) is not unique ``` because the bound arrives as an untyped literal. The dataset editor catches it — the preview shows **"Filters will not be mirrored"** and quotes the engine error — but Explore still shows the glyph, and the SQL contains no `dt_epoch`. `extract(epoch from cast(:value as timestamp))` is the working form. The editor warning is a decent guardrail, but the indicator can still be wrong for anyone who saved a bad transform and did not look at the preview. Flagging for the reviewer rather than fixing here: closing it means either evaluating the transform in the summary (expensive, on a hot path) or persisting the last probe result and reading it back. #### UX observations - The glyph costs about 16px inside the chip and truncates the label at the default control-panel width: `2026-01-01 ≤ event_time < 2026-0…` with the glyph, versus the full range without it. - Unrelated pre-existing behaviour worth knowing if you reproduce this: Calendar Heatmap throws `Please provide both time bounds (Since and Until)` when `time_range` is `No filter`, so the third row of the Time Range table above shows a chart-level data error. The control panel — the part under test — renders correctly. #### Screenshots `.context/` is gitignored, so these are attached to the PR by hand: | File | Shows | | --- | --- | | `01-dataset-editor-mapping-state-a.png` | `lower(:value)`, ordering unchecked, preview `country_key IN ('us', 'ca')` | | `02-adhoc-operator-matrix.png` | all eight chips, glyph on exactly two | | `03-tooltip.png` | the tooltip, open | | `04-view-query-mirrored-predicate.png` | both `country_key` mirrors in the SQL | | `05-dataset-editor-mapping-state-b.png` | `dt_epoch` ← `event_time`, ordering checked, preview valid | | `06-temporal-range-chip.png` | glyph on the temporal range chip | | `07-temporal-range-view-query.png` | the two `dt_epoch` range predicates | | `08-temporal-range-no-filter.png` | no glyph on `No filter` | | `09-time-control-mirrored.png` | glyph beside the standalone Time Range control | | `10-time-control-other-column.png` | no glyph when Time Column is `ingested_at` | | `11-time-control-no-filter.png` | no glyph when Time Range is `No filter` | | `12-known-gap-editor-transform-invalid.png` | editor: "Filters will not be mirrored" | | `13-known-gap-glyph-with-invalid-transform.png` | Explore: glyph shown anyway | | `14-known-gap-view-query-no-mirror.png` | the SQL that has no mirror | | `15-flag-off-no-glyph.png` | flag off, glyph gone | | `16-flag-restored-glyph-back.png` | flag restored, glyph back | ### Deviation from the mockup 1d shows the tooltip ending in a clickable **View generated SQL →** link. This ships as static text pointing at *View query* instead: the tooltip hangs off a small hover glyph, so reaching a link inside it means traversing from the icon without dismissing the tooltip, and the View query action lives in the chart's `⋯` menu rather than anywhere the control panel can reach without new plumbing. Happy to add the link if the interaction is worth the wiring. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [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 - [x] 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]
