sadpandajoe opened a new pull request, #45855:
URL: https://github.com/apache/superset/pull/45855
### SUMMARY
Follow-up to #43870 (partition filter mapping). With
`PARTITION_FILTER_MAPPING` on, a dataset's partition column is resolved from
the dataset's stored column list. If that column is dropped or renamed in the
warehouse, the stored list still has it until someone runs **Sync columns from
source**, so the mirrored predicate is still added to every time-ranged query.
The warehouse rejects the statement ("invalid column reference" and similar),
and every time-ranged chart on the dataset fails, including charts that never
select the partition column.
The mirror is only an optimisation: the original time filter already selects
the same rows, so dropping it changes speed, not results. This change retries
the query once without the mirror when the warehouse rejected it for that
reason.
How the retry decides:
- It only considers a warehouse failure: a raw driver exception, or a
`SupersetDBAPIError` that an engine spec mapped one into. Superset's own
timeout, security, cancellation, validation and OAuth errors, and
`BaseException`s such as a worker deadline, are never retried.
- The error message must name an identifier the mirror emitted: the
partition column, plus the columns its expression reads when it is a calculated
column. Matching is case-insensitive and treats only letters, digits, `_` and
`$` as part of a name, so `my_dt_key` is not `dt_key`.
- A mapping must resolve, and the query rebuilt without the mirror must
differ from the one that failed.
- If the retry fails too, its error is the one reported, and the reported
SQL and column labels are those of the statement that last ran. If the decision
itself cannot be made, the original error is reported unchanged.
The documentation gets a short section describing this, including what it
does not cover: errors whose message does not name the column, queries that
select the missing column themselves (Samples, Drill to detail), streaming
exports and cursor-paginated Samples pages, and the pruning indicator, which
keeps reading the stored columns.
## Root Cause
`resolve_partition_mapping()` and `build_mirrored_predicates()` only look up
the partition column in `datasource.columns`, which is cached metadata, so a
column removed at the source still resolves. `ExploreMixin.get_sqla_query()`
then appends the mirrored predicate to every time-ranged query, and
`ExploreMixin.query()` turns the warehouse rejection into a failed result with
no fallback. The only code that clears a dangling mapping
(`DatasetDAO.clear_dangling_partition_mapping`) runs on write paths such as the
column sync, never when a chart is queried. Existing mirror tests compile SQL
or stub the database, so none executed a mirrored statement against a table
that lacks the column.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: backend change with no UI change (the documentation page
gains one section).
Regression guards run real queries through `SqlaTable.query()` against
SQLite tables:
- Before the fix, a time-ranged chart on a dataset whose partition column is
gone fails (`no such column: dt_key`), for a dropped column, a renamed column,
and a calculated partition column whose source column is gone (`no such column:
source_key`).
- After the fix, those charts return the correct rows from an unpruned
statement. Controls show the mirror is still emitted when the column exists.
- Further tests pin which failures are retried (raw and mapped driver
errors, identifiers spelled with `$`, quoted or upper-case names) and which are
not (Superset timeout and security errors, validation errors, errors naming a
different identifier such as `my_dt_key`), that a failing retry reports its own
error, and that a series-limited chart recovers.
### TESTING INSTRUCTIONS
Automated:
- `pytest tests/unit_tests/models/partition_mirroring_test.py
tests/unit_tests/connectors/sqla/partition_mapping_test.py`
Manual:
1. Enable `PARTITION_FILTER_MAPPING` and map a time column to a partition
column on a dataset over a warehouse table.
2. Drop or rename the partition column in the warehouse without syncing the
dataset's columns.
3. Open a chart on the dataset with a time range. It should load, with the
unpruned query visible in **View query**.
4. Run **Sync columns from source**. The mapping is cleared and the dataset
editor says so.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `PARTITION_FILTER_MAPPING`
- [ ] 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
- [ ] 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]
