sadpandajoe opened a new pull request, #42093: URL: https://github.com/apache/superset/pull/42093
### SUMMARY `superset/semantic_layers/mapper.py` defined `_get_time_axis_column` twice, with deliberately different semantics: - **offset resolver** (defined first): returns the first temporal column, falling back to the column named in a `TEMPORAL_RANGE` adhoc filter, so aggregate-only charts that carry the temporal column only in a filter still get offset-aware time bounds. Used by `_get_time_filter` and `_get_group_limit_filters`. - **grain resolver** (defined later): only claims an axis when the selected columns contain exactly one temporal dimension, otherwise returns `None` so the grain path falls back to raw variants instead of guessing. Used by `map_query_object` and `_validate_granularity`. Because both shared the name `_get_time_axis_column`, the second definition shadowed the first. Two consequences: 1. **mypy failed pre-commit** — `Name "_get_time_axis_column" already defined [no-redef]` — turning the branch red. 2. **Silent regression**: the offset call sites resolved to the grain function at runtime, so the time offset / time-comparison bounds were dropped for aggregate-only charts whose temporal column lives only in a `TEMPORAL_RANGE` filter. The unit test covering that path was already failing but stayed hidden because an unrelated Python 3.10 CI leg errored out first and cancelled the sibling jobs. This PR renames the grain resolver to `_get_grain_time_axis_column` and points its call sites (`map_query_object`, `_validate_granularity`) at it, leaving the offset resolver under the original name for its call sites. The two duplicated test blocks are reconciled so both resolvers are covered. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/semantic_layers/mapper_test.py ``` 185 tests pass. Before this change, `test_get_time_axis_column_finds_temporal_in_temporal_range_filter` failed (the masked regression) and `pre-commit run mypy` reported the `[no-redef]` error; both pass now. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] 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 > **Note on red CI checks:** The failing **Python 3.10** unit/integration legs on this PR (and the resulting cancellation of the 3.11 / 3.12 legs) are a pre-existing `syntaqlite` / `StrEnum` incompatibility unrelated to this change, already tracked separately (#42007 / #42045 / #42058). This PR is intentionally scoped to the `mapper.py` regression above; the **pre-commit** check reflects the actual fix here. -- 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]
