aminghadersohi opened a new pull request, #44435:
URL: https://github.com/apache/superset/pull/44435
### SUMMARY
Passing `time_grain_sqla` through `extra_form_data` to the MCP
`get_chart_data` /
`get_chart_sql` tools was silently ignored for any chart built on Generic
Chart
Axes (anything with an `x_axis` — `echarts_timeseries_bar`,
`mixed_timeseries`,
etc.). The tools returned the chart's *saved* grain: identical row counts and
timestamps to a call with no override at all, and no error, so an automated
consumer had no way to detect that the request had been dropped.
**Root cause.** Generic Chart Axes charts carry their temporal x-axis as a
`BASE_AXIS` adhoc column inside the query's `columns`, and that column's own
`timeGrain` is what actually becomes the SQL truncation
(`SqlaTable.adhoc_column_to_sqla` reads `col["timeGrain"]`; `has_timegrain`
requires `columnType == "BASE_AXIS"`). It also wins in
`ExploreMixin.get_time_grain`, which prefers `columns[0]["timeGrain"]` over
`extras`. The MCP merge path
(`merge_form_data_filters_into_query`) wrote the override only into
`extras["time_grain_sqla"]`, which those queries never consult — so the saved
grain kept winning.
**Fix.** Propagate the override onto the `BASE_AXIS` column as well,
mirroring
what `apply_dashboard_filter_context` already does for the dashboard filter
path. Queries without a `BASE_AXIS` column are unaffected and keep using
`extras`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — no UI change.
### TESTING INSTRUCTIONS
New regression tests in
`tests/unit_tests/mcp_service/chart/test_time_grain_generic_axes.py` execute
the
resulting query against a real SQLite dataset (90 consecutive daily rows), so
the override is proven by the returned buckets rather than by an echoed
field:
```
pytest tests/unit_tests/mcp_service/chart/test_time_grain_generic_axes.py
```
- A saved `P1D` chart overridden with `P1M` returns 3 monthly buckets
(`2024-01-01`, `2024-02-01`, `2024-03-01`) instead of 90 daily ones.
- The reverse direction (saved `P1M`, overridden to `P1D`) returns 90 rows.
- Non-axis adhoc columns are left untouched, and a request with no grain
override preserves the saved axis grain.
On `master` the first three of those tests fail: the overridden query returns
the same 90 daily rows as the baseline.
Manual check: call the MCP `get_chart_data` tool on an
`echarts_timeseries_bar`
chart saved with a daily grain, passing
`extra_form_data={"time_grain_sqla": "P1M"}`, and confirm the returned rows
are
monthly.
### 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
--
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]
