hughhhh opened a new pull request, #45562:
URL: https://github.com/apache/superset/pull/45562
### SUMMARY
Stacked on #43870 (base `hughhhh/pfm-6-explore-indicator`). Addresses the
review comment on lifecycle boundaries: *ID-preserving renames can discard
mapping references, unused mapping fields make exports fail older import
schemas, and invalid references stored with the flag off can block unrelated
saves after enablement.* Each one was reproduced against a real SQLite metadata
DB before fixing.
**1. ID-preserving renames.** `partition_column` / `partition_mapped_column`
store column *names*, while `DatasetDAO._upsert_columns` renames a column in
place by id. A `PUT` that renamed:
- the partition column → `clear_dangling_partition_mapping` nulled the whole
mapping (and, flag on, wiped the mapped column's transform);
- the mapped column → the override was nulled, so the mapping silently
re-pointed at `main_dttm_col` and the renamed column's transform was dropped.
New `follow_partition_column_renames` (in `partition_mapping.py`) rewrites a
reference to the column's new name when the request omits the reference or
echoes the stored name back. The echo is what the dataset editor sends; before
this, an editor rename of the mapped column was rejected with "Mapped column X
is not a column". It runs in `DatasetDAO.update` and in `UpdateDatasetCommand`
validation. A reference the request sets to a different column is respected.
`override_columns=true` is skipped, because that path matches columns by name.
**2. Export compatibility.** `export_to_dict(include_defaults=True)` wrote
all four mapping fields into every dataset export, even as `null`/`false`. An
older Superset's `ImportV1DatasetSchema` rejects the whole bundle with `Unknown
field.`. The export now leaves them out while they hold their defaults. The v1
importer reads an omitted field as its default, because `import_from_dict`
applies only the keys present: without that, overwriting a mapped dataset with
a bundle from an unmapped one would have kept the mapping.
**3. Flag-off writes blocking later saves.**
`UpdateDatasetCommand._validate_partition_mapping` and the legacy
`/datasource/save` reference check returned early with the flag off. So a `PUT`
could store a dangling reference or a non-deterministic/Jinja transform, and
once the flag went on, every save failed over it, including description-only
and metrics-only edits. The importer's blocking transform checks were
flag-gated too. Now:
- storing a mapping is validated whether or not the flag is on (REST PUT,
legacy save, import);
- a save is refused only over a blocking issue the *request* introduces:
issues already present in the stored mapping are compared and forgiven, and the
transform expression gate is skipped for the unchanged stored transform on the
same source. A dataset stored broken can still be edited, and an editor that
echoes the stored mapping never trips the now-ungated check on a deployment
that never enables the flag.
### BEHAVIOUR CHANGES
- Flag off: a request that *introduces* a broken mapping is now a 422
(previously stored silently).
- A v1 bundle without the mapping fields (an older bundle, or an export of
an unmapped dataset) imported with overwrite now clears the target's mapping,
instead of keeping it.
- Not covered: renaming `main_dttm_col` in place while the mapping follows
it (no override). That column reference is upstream behaviour and isn't
followed on master either.
### TESTING INSTRUCTIONS
New unit tests:
- `dao/dataset_test.py`: in-place rename of either end, with the reference
omitted or echoed; an explicit reference is respected; `override_columns` is
skipped.
- `commands/dataset/update_test.py`: description-only and metrics-only saves
survive each kind of stored-broken mapping; a new problem is still refused next
to a stored one; the editor's echo after a rename validates.
- `views/datasource/views_test.py`: the legacy save refuses a reference the
request breaks (flag on and off) and survives one already broken in storage.
- `datasets/commands/export_test.py` and `importers/v1/import_test.py`:
export only when set; overwrite reads omitted fields as defaults; an unsaveable
transform is dropped with the flag off.
Four existing tests were updated to the new behaviour, and the export golden
test loses the `null` mapping lines. I also checked by hand that an unmapped
dataset's export loads under master's `ImportV1DatasetSchema`.
Ran
`tests/unit_tests/{commands/dataset,dao/dataset_test.py,datasets,views/datasource,connectors/sqla,dashboards,models/partition_mirroring_test.py,mcp_service/dataset}`:
1962 passed. Ruff and CI-pinned mypy 1.15 are clean on the changed files.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags: `PARTITION_FILTER_MAPPING`
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] 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]
