hughhhh opened a new pull request, #43757:
URL: https://github.com/apache/superset/pull/43757
### SUMMARY
First of a four-PR stack adding **partition filter mapping**. This one adds
the storage and the save-time validation; nothing reads the mapping yet.
Datasets on Hadoop-family engines are often partitioned on a *technical*
column — an epoch integer, a lowercased region key — that no analyst would ever
filter on. Unless a query carries a predicate on that column the engine scans
every partition, and today the only workaround is hand-writing the predicate as
custom SQL in a virtual dataset, which pushes a performance concern onto every
chart author and takes the dataset out of the physical/syncable path.
**Storage.** Four columns, following the `always_filter_main_dttm` /
`currency_code_column` precedent for "a dataset-level setting that names a
column":
| Column | Purpose |
|---|---|
| `tables.partition_column` | the physical partition column |
| `tables.partition_mapped_column` | explicit override; `NULL` follows
`main_dttm_col` |
| `table_columns.partition_value_transform` | the `:value` expression |
| `table_columns.partition_transform_is_monotonic` | gates range mirroring
(see PR 2) |
Effective mapped column is `partition_mapped_column or main_dttm_col`, which
makes "re-pointing the default datetime column moves the mapping with it,
unless overridden" fall out of the model rather than needing code.
The monotonic flag is `NOT NULL DEFAULT false` rather than a nullable
tri-state, matching `normalize_columns` — a nullable boolean invites `if x:`
bugs where `None` and `False` need distinguishing and don't get it.
A JSON blob inside `tables.extra` was considered and rejected: `extra` is a
user-editable free-text box in the dataset editor, `buildExtraJsonObject`
rebuilds `column.extra` from a hardcoded key list on every save, and there is
no validation layer for it today. Every comparable setting shipped in the last
year chose a real column.
**Validation** runs in two tiers, because the PRD wants a mapping to "stay
inactive until it parses" while some errors must still hard-block:
- **Blocks the save:** unknown columns, a column mapped onto itself, Jinja
in the transform, non-deterministic functions.
- **Saves, mapping inactive:** unparseable transform, transform missing
`:value`, no transform.
Two details worth a look:
- The self-mapping check validates the **effective** mapped column. Checking
only the explicit override misses the case an owner actually hits — pointing
`partition_column` at the column that is already `main_dttm_col`.
- `SQLStatement.get_niladic_functions` is added because the denylist can't
be purely name-based: on Hive and Impala `unix_timestamp()` means "now" while
`unix_timestamp(x)` — the canonical transform for this feature — is pure. Note
sqlglot's Hive dialect already resolves the zero-arg form to
`CURRENT_TIMESTAMP`; this is the backstop for dialects that don't normalize.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no UI in this PR.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/connectors/sqla/partition_mapping_test.py \
tests/unit_tests/datasets/partition_mapping_serialization_test.py \
tests/unit_tests/sql/parse_tests.py \
tests/unit_tests/commands/dataset/update_test.py \
tests/unit_tests/datasets/commands/export_test.py
```
Migration, verified on sqlite in both directions:
```bash
superset db upgrade # adds all four columns to tables/table_columns +
both _version shadow tables
superset db downgrade 1072de5ed955
```
The serialization tests assert the mapping survives every layer it passes
through (`export_fields`, the `data` payload, PUT/import-v1 schemas, the API
column lists) — a field missing from any one of them is dropped silently, which
is the failure mode they exist to catch.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `PARTITION_FILTER_MAPPING` (off by default)
- [ ] Changes UI
- [x] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [x] Confirm DB migration upgrade and downgrade tested
- [x] Runtime estimates and downtime expectations provided — four `ADD
COLUMN`s with no backfill and no index; effectively instant on Postgres/MySQL,
no downtime expected
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
