hughhhh opened a new pull request, #43891: URL: https://github.com/apache/superset/pull/43891
### SUMMARY Fourth of the partition filter mapping stack, and the PR that makes the feature reachable. Until now a mapping could only be configured with a `PUT /api/v1/dataset/<pk>`, and the preview endpoint added in the previous PR had no caller — its own commit message says the visual controls "are not in this PR". This implements them: wireframes **1a, 1b, 1c, 1g, 1h**. **Includes the documentation** (`docs/admin_docs/configuration/partition-filter-mapping.mdx`, originally #43760, now closed). The two are combined deliberately: that page tells operators to pick a **Partition column** in the dataset editor and refers to "the editor's preview panel", so shipping it ahead of these controls would publish instructions for things that do not exist yet. Merging them together removes that window entirely. Stacked on `hughhhh/pfm-3-preview-and-editor`. Review the [compare against pfm-3](https://github.com/apache/superset/compare/hughhhh/pfm-3-preview-and-editor...hughhhh/pfm-5-editor-ui) rather than the diff against master. **What's new in the editor** - **Partition column** select in the Columns tab's Default Column Settings, plus a read-only computed **Maps to partition**. It's deliberately not a peer dropdown — it reflects `partition_mapped_column ?? main_dttm_col`, and a second select would let it drift from the default datetime column silently. - The partition column's row is muted, carries a `PARTITION` tag, and defaults `Is filterable` / `Is dimension` off — still manually togglable. - A **Partition filter mapping** section in the row expand with three states: the mapped column holds the transform, every other column offers to take the mapping over, and the partition column itself shows nothing. - A live **Preview** panel showing sample input → emitted predicate, and a destructive **Remove mapping**. **Two places the spec and the merged backend disagreed** 1. The mockups have no monotonicity control, but the query path only mirrors ranges when the transform is declared order-preserving — so 1d/1e's own headline example (a time range producing two `dt_epoch` bounds) was unreachable through the UI. This adds the **Transform preserves ordering** checkbox that the docs in pfm-4 already describe. 2. The PRD asks for `unix_timestamp(:value)` as the temporal default, but that's Hive syntax and would not parse on Postgres, Trino or BigQuery. The default moves to the engine spec (`partition_value_transform_default`, set on Hive/Impala/Spark); engines without one offer no pre-fill rather than a wrong one. **Backend changes** - The preview endpoint takes `sample_values` + an `operator` and builds its predicate with `build_mirrored_predicates` — the same function the query path uses — so what the panel shows is what a chart emits, `IN` included. It also accepts a candidate `partition_column`, because the editor previews a mapping the owner hasn't saved yet and a preview that requires saving first isn't a preview. - Failed probes carry the engine's own message through an opt-in `errors` sink (the hot query path passes nothing and stays silent). sqlglot parses unknown functions happily, so a misspelled one is an *engine* error and was previously reported as an unexplained blank. - Parse failures now name a position, mapped back through the `SELECT` prefix and the `:value` → `NULL` substitution so it points at what the owner actually typed. **Two silent read-path bugs this uncovered** Neither was reachable from unit tests; both needed the running app. - `columns.partition_value_transform` and its monotonic flag were in the model, the export fields and the PUT schema but **not** in `show_columns` — so the editor reopened a saved mapping as if it had none, *and the next save wrote that emptiness back*. Related-model fields must be listed in `show_columns`, not only `show_select_columns` (`columns.advanced_data_type` is in both for the same reason). - `partition_value_transform_default` needed the same treatment to reach the pre-fill. Both are now pinned by tests in `partition_mapping_serialization_test.py`, whose whole premise is that a field missing from any one layer is dropped without a sound. **One small shared change:** `Field` gains an opt-in `passItemToControl`. The row-expand section keys off the whole column record, not just the transform it edits, and handing an unknown `item` prop to every `TextControl` and `Select` wasn't worth the convenience. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF _Screenshots captured during validation; attaching separately._ - `1a` Partition column + Maps to partition, and `1b` the muted `PARTITION` row - `1c` Row expand with transform, ordering checkbox and a valid preview - `1c` Parse error state — only one of preview/error is ever visible - `1g` Partition column set with nothing mapped - `1h` Non-temporal `country → region_key` ### TESTING INSTRUCTIONS Automated: `1800` backend unit tests and `225` frontend Datasource tests pass; `tsc`, `ruff`, `ruff format` and `oxlint` are clean (oxlint warnings in `src/components/Datasource` go 32 → 30, since the duplicated column-name renderer was folded into one helper). Manually, against a real engine: 1. Enable the flag: `FEATURE_FLAGS = {"PARTITION_FILTER_MAPPING": True}`. 2. Create a physical dataset on a table with both a business column and a partition column, where the partition column really is the transform of the other. On Postgres: ```sql CREATE TABLE partition_demo (event_time TIMESTAMP, dt_epoch BIGINT, country TEXT, region_key TEXT, revenue DOUBLE PRECISION); INSERT INTO partition_demo SELECT ts, EXTRACT(epoch FROM ts)::bigint, t.c, lower(t.c), (random()*100)::numeric(10,2) FROM generate_series(timestamp '2026-07-01', timestamp '2026-08-15', interval '6 hour') ts, (VALUES ('US'),('CA'),('MX')) AS t(c); ``` 3. Edit the dataset → **Columns** → set **Partition column** to `dt_epoch`. Confirm *Maps to partition* shows `event_time` tagged *Default datetime column*, and that `dt_epoch`'s row is muted with a `PARTITION` tag and its toggles off. 4. Follow **Map a different column instead →**. Set the value transform to `cast(extract(epoch from cast(:value as timestamp)) as bigint)` and check **Transform preserves ordering**. The preview should read `event_time >= '2026-01-15 00:00:00'` → `dt_epoch >= 1768435200`. Unchecking it drops the preview back to `=`. 5. Break the transform (`lower(:value))`) → *Can't parse transform* with a position, and the preview panel disappears. 6. Save, reopen — everything round-trips. 7. Build a chart with a time range and open **View query**: ```sql WHERE event_time >= TO_TIMESTAMP('2026-07-01 …') AND event_time < TO_TIMESTAMP('2026-08-01 …') AND dt_epoch >= 1782864000 AND dt_epoch < 1785542400 ``` 8. Non-temporal: point **Partition column** at `region_key`, move the mapping to `country`, set `lower(:value)`. Preview shows `country IN ('US', 'CA')` → `region_key IN ('us', 'ca')`, and a chart filtered on `country` emits `AND region_key IN ('us', 'ca')`. 9. Negative: turn the flag off. The controls disappear, the preview endpoint 404s, and no mirrored predicate is added. **Known limitation, not introduced here:** SQLAlchemy's `text()` misparses Postgres `::` casts, so a transform written as `:value::timestamp` leaves the placeholder unbound. This comes from `build_probe_sql` in pfm-2 and is harmless on Hive/Impala, which have no `::` syntax — the ANSI `cast(:value as timestamp)` form works. Worth a follow-up for Postgres/Redshift users. ### 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) --- ## How Has This Been Tested? Validated against the full stack under Docker (Flask + Celery worker + Postgres + Redis) serving this branch, driving the real browser. Two rounds: once during implementation, once after pushing — the second round found the three defects fixed in `8f873f3`. **Bring-up** (non-default ports, since 8088/5432 are usually taken by another stack): ```bash printf 'FEATURE_FLAGS = {"PARTITION_FILTER_MAPPING": True}\n' > docker/pythonpath_dev/superset_config_docker.py SUPERSET_PORT=8090 CYPRESS_PORT=8091 DATABASE_PORT=5442 REDIS_PORT=6389 NODE_PORT=9010 \ docker compose -p kingston-pfm up -d db redis superset-init superset superset-worker superset-node until curl -sf -o /dev/null http://localhost:8090/health; do sleep 5; done ``` **Fixture** — a table where the partition key really is the transform of the business column, so a mirrored predicate is verifiably correct rather than merely present: ```sql CREATE TABLE partition_demo (event_time TIMESTAMP, dt_epoch BIGINT, country TEXT, region_key TEXT, revenue DOUBLE PRECISION); INSERT INTO partition_demo SELECT ts, EXTRACT(epoch FROM ts)::bigint, t.c, lower(t.c), (random()*100)::numeric(10,2) FROM generate_series(timestamp '2026-07-01', timestamp '2026-08-15', interval '6 hour') ts, (VALUES ('US'),('CA'),('MX')) AS t(c); ``` **The walk** (543 rows, 5 columns): | # | Step | Result | |---|---|---| | 0 | Open the dataset editor → **Columns** | `00-before-no-partition-column.png` — Partition column `None`, no *Maps to partition*, all rows normal | | 1 | Set **Partition column** = `dt_epoch` | `01-1a-1b-partition-column-selected.png` — *Maps to partition* → `event_time` tagged *Default datetime column*; `dt_epoch` row muted with `PARTITION` tag and its three toggles off, every other row untouched | | 2 | **Map a different column instead →**, set the transform, tick **Transform preserves ordering** | `02-1c-row-expand-valid-preview.png` — preview `event_time >= '2026-01-15 00:00:00'` → `dt_epoch >= 1768435200`. Unticking drops it back to `=` | | 3 | Break the transform (`lower(:value))`) | `03-1c-parse-error.png` — *Can't parse transform*, **syntax error at position 14**, preview hidden (only one of preview/error is ever visible) | | 4 | Clear the default datetime column | `04-1g-partition-column-no-mapping.png` — `No mapping` chip, *Map a column →*, and the scan-every-partition warning | | 5 | Partition column → `region_key`, move the mapping to `country`, `lower(:value)` | `05-1h-non-temporal-mapping.png` — required asterisk, `country IN ('US', 'CA')` → `region_key IN ('us', 'ca')` | | 6 | Save and reopen | Everything round-trips, including the transform and the monotonic flag | Screenshots live in `.context/pfm-ui-screenshots/` (gitignored) and are attached above. **Non-visual proof.** The generated SQL, via `/api/v1/chart/data` with `result_type: query` — the same SQL the *View query* panel renders: ```sql -- temporal mapping, Explore time range: both bounds mirrored WHERE event_time >= TO_TIMESTAMP('2026-07-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND event_time < TO_TIMESTAMP('2026-08-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND dt_epoch >= 1782864000 AND dt_epoch < 1785542400 -- non-temporal mapping, categorical filter: mirrored element-wise WHERE country IN ('US', 'CA') AND region_key IN ('us', 'ca') ``` `1782864000` is exactly `2026-07-01T00:00:00Z`, matching how the fixture populates `dt_epoch` — so the pruning predicate selects the same rows rather than merely looking plausible. **Negative case** — with `PARTITION_FILTER_MAPPING` off: the Partition column field, the `PARTITION` tag and the muted row all disappear, the preview endpoint returns **404**, and the same query emits no `region_key` predicate at all. Also verified the engine-specific default behaves: on Postgres `partition_value_transform_default` is `None`, so the editor offers no pre-fill rather than a Hive expression that would not parse. ### What the browser pass caught that tests did not Worth calling out, since it's the argument for doing this at all: - **Saved mappings did not load back** — the per-column fields weren't in `show_columns`, so reopening showed an empty transform and the next save persisted that emptiness. Fixed, and pinned by tests. - **The preview required saving first**, since it read the stored partition column rather than the candidate. - **Three layout/heuristic defects** fixed in `8f873f3`: the engine's error text clipped mid-sentence (expanded antd rows size to content, and alert descriptions are `nowrap` in this theme), and *Map a column →* suggesting `revenue` — inviting an owner to mirror a currency metric onto a region key. ### Additionally verified (third pass) - **The partition column really does leave Explore's pickers.** The payload Explore's controls consume (`GET /api/v1/explore/?datasource_type=table&datasource_id=…`) carries `region_key: {groupby: false, filterable: false}` while every other column stays `true`; `dndControls.tsx` builds the Dimensions options from `columns.filter(c => c.groupby)` and the Filters options from `columns.filter(c => c.filterable)`. Both ends verified against the running app. I could not get the drag-and-drop popover itself to open under automation, so this rests on the payload plus Superset's existing filter rather than on my having seen the rendered list. - **A column sync clears a dangling mapping.** Dropped `region_key` from the physical table with the mapping live, then hit **Sync columns from source**: three toasts fire — `Metadata has been synced`, **`The partition filter mapping was cleared: its column is gone`**, `Removed 1 column from the virtual dataset` — the Columns badge goes 5 → 4, and *Partition column* resets to `None` with *Maps to partition* gone. `06-sync-clears-dangling-mapping.png` captures the resulting state (the toasts auto-dismiss before a screenshot lands; the strings above are read straight from the DOM). ### Wireframe 1e in the real panel **View query** shows the mirrored predicate as an ordinary `WHERE` clause, exactly as 1e specifies — captured in `07-1e-view-query-mirrored-predicate.png`: ```sql SELECT country AS country, SUM(revenue) AS "SUM(revenue)" FROM public.partition_demo WHERE event_time >= TO_TIMESTAMP('2026-07-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND event_time < TO_TIMESTAMP('2026-08-01 00:00:00.000000', 'YYYY-MM-DD HH24:MI:SS.US') AND dt_epoch >= 1782864000 AND dt_epoch < 1785542400 GROUP BY country ORDER BY "SUM(revenue)" DESC LIMIT 100 ``` ### Known limitation (pre-existing, not introduced here) SQLAlchemy's `text()` misparses Postgres `::` casts, so a transform written `:value::timestamp` leaves the placeholder unbound and the probe fails with *"This text() construct doesn't define a bound parameter named 'value'"*. It comes from `build_probe_sql` in pfm-2 and is harmless on Hive/Impala, which have no `::` syntax. The ANSI form `cast(:value as timestamp)` works and is what the walk above uses. Worth a follow-up for Postgres/Redshift. -- 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]
