hughhhh opened a new pull request, #45546:
URL: https://github.com/apache/superset/pull/45546
### SUMMARY
Stacked on #43870. Addresses the review note there that calculated partition
expressions bypass template processing.
**The bug.** `build_mirrored_predicates` builds the partition column with
`convert_tbl_column_to_sqla_col(partition_column)` and no template processor.
When the partition column is a calculated column whose expression holds Jinja,
that expression goes into the predicate unrendered: the query-time validator
can't parse `{{ ... }}`, so it skips the check and returns the text as-is.
Reproduced with `dt_calc = {{ 'dt_' ~ 'epoch' }}`:
- Preview returns `valid: true` with `emitted_predicate` set to `({{ 'dt_' ~
'epoch' }}) = 1768435200 OR ({{ 'dt_' ~ 'epoch' }}) IS NULL`.
- The chart SQL renders the same column as `(dt_epoch)` in `SELECT` and
`GROUP BY`, but leaves it raw in the mirrored `WHERE`, so the query fails at
the engine.
Before this change, only the value transform was checked for Jinja.
**The fix: reject it rather than render it.** Rendering with the query's
template processor would not help:
- Preview has no chart template context (`url_params`, `filter_values`,
current user), so it couldn't show what a chart actually emits.
- A key computed per request isn't a physical partition, so the engine can't
prune on it anyway.
Changes:
- `validate_partition_mapping` takes a new `partition_expression` argument
and reports a blocking issue on `partition_column` when it contains Jinja.
- The REST update command (reads the payload, falling back to the stored
column), the legacy `/datasource/save/` path, and preview all pass the
expression in. `_effective_transform` is generalized to
`_effective_column_attribute` for this.
- `resolve_partition_mapping` declines such a column at query time, to cover
imports and rows saved before this check. The Explore summary reports the
mapping as inactive.
A calculated partition column without Jinja still mirrors as before.
### TESTING INSTRUCTIONS
Run the partition-mapping, dataset command, datasource view, model and MCP
dataset unit suites. Locally: 2,523 passed, ruff is clean, and mypy reproducing
the CI hook (`--check-untyped-defs`) is clean on the touched files.
New tests cover each layer:
- validator: expression, statement and comment Jinja forms
- `resolve_partition_mapping`: templated vs. untemplated partition column
- REST update: stored and payload expressions
- legacy save: returns 422
- preview: returns invalid without calling the probe
- Explore summary: reports inactive
### 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]
