hughhhh opened a new pull request, #43758:
URL: https://github.com/apache/superset/pull/43758
### SUMMARY
Second of four, stacked on the model/migration PR. Reads the mapping and
appends an equivalent predicate on the partition column to every query, so
chart authors change nothing and queries prune.
**Operator safety is the whole of this change.** The PRD says "comparison
and range operators map directly; `IN` maps element-wise" — that is not sound
in general, and the unsoundness is silent, showing up as wrong chart numbers
rather than an error. A mirrored predicate `P2` may only be `AND`-ed onto the
query when the original `P1` *implies* it:
| Original | Safe when |
|---|---|
| `col = v`, `col IN (…)` | **always** — `T` is a function |
| `col >=`, `>`, `<`, `<=`, `TEMPORAL_RANGE` | **only if `T` is monotonic** |
| `col != v`, `NOT IN`, `LIKE`, `IS NULL`, … | **never** |
Negations are never safe because `T` need not be injective: `lower(:value)`
with `country != 'US'` mirrors to `region_key != 'us'`, which excludes rows
whose `country` is already lowercase `'us'` — rows the original filter *keeps*.
**Monotonicity is declared, not inferred.** An earlier draft inferred it
from the column type ("a temporal→partition-key transform is monotonic by
construction"). That's wrong, and it fails on partition schemes people actually
use: `hour(:value)`, `date_format(:value, 'dd')` and `dayofweek(:value)` are
all reasonable transforms on a `TIMESTAMP` column and none preserve ordering.
Since the Explore time range is a *range* operator, this is not an edge case.
`T(lower) <= T(upper)` is asserted at runtime as a backstop — necessary, not
sufficient, and free because both bounds are already being resolved.
**Two things worth reviewer attention:**
1. `get_time_filter` applied the dataset timezone and legacy hour offset to
the bounds *internally*. That adjustment is extracted into `adjust_time_bounds`
so the mirrored predicate is resolved from the **same instants** the timestamp
predicate compares against. Probing the raw bounds would be wrong by exactly
the offset, silently; duplicating the logic would drift.
2. **`PARTITION_FILTER_MAPPING` must be configured as a static boolean.**
`FEATURE_FLAGS` also accepts per-request callables, and since the mapping only
enters the cache key when active, a flag resolving per user would let a
flag-*off* user read a cache entry written from pruned SQL by a flag-*on* user.
**Safety posture.** Values are bound, never interpolated — they're
attacker-controlled in the sense that a Gamma user picks filter values. The
probe query references no tables, so it can't leak data or bypass RLS. Any
failure at all falls open to no pruning: the query still runs and is still
correct, it just scans more partitions.
**Deliberate non-coverage,** documented in PR 4: filter-value dropdowns
(`values_for_column` builds its own `SELECT DISTINCT` and never reaches this
code path), RLS predicates and `extras.where` (raw SQL appended downstream of
the structured-filter loop), and columns with an active `advanced_data_type`
(`translate_filter` builds its own predicate shape from *translated* values, so
the `(operator, value)` pair this reasoning depends on doesn't exist).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
The mirrored predicate lands in `where_clause_and` and shows up in **View
query** as an ordinary `WHERE` clause, by design:
```sql
WHERE event_time >= '2026-01-01 00:00:00'
AND event_time < '2026-02-01 00:00:00'
AND dt_epoch >= 1767225600
AND dt_epoch < 1769904000
```
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/models/partition_mirroring_test.py \
tests/unit_tests/connectors/sqla/partition_mapping_test.py
```
The mirroring tests stub the engine probe and assert on the generated SQL,
so they pin down *which* predicates are emitted and with what values — where
the correctness argument lives. Worth reading as the review surface: nothing
emitted for `!=`/`NOT IN`/`LIKE`/`IS NULL`; nothing for a range when the
transform isn't declared monotonic; nothing when `T(lo) > T(hi)`; no duplicate
when `granularity` and a `TEMPORAL_RANGE` filter hit the same column; and the
timezone/hour-offset tests, which catch the whole class of off-by-offset bugs.
There is **no UI to configure a mapping yet** — this is exercised through
the ORM.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `PARTITION_FILTER_MAPPING` (off by default)
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [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]
