AryaKetanShCt opened a new pull request, #42927:
URL: https://github.com/apache/superset/pull/42927
### SUMMARY
Fixes the first symptom of #42926.
A chart's `query_context` is written when the chart is saved and is never
rewritten. Explore rebuilds the query from `form_data` at every render and
never reads it, so only the paths that are not a browser replay it: `GET
/api/v1/chart/<id>/data/`, alerts and reports, thumbnails, cache warm-up, CSV
export.
The stored query therefore ages while the engine moves on. `pivot` used to
accept `flatten_columns` and `reset_index`; flattening became its own operation
and those parameters were removed. `exec_post_processing` passes the stored
options straight through:
```python
df = getattr(pandas_postprocessing, operation)(df, **options)
```
so replaying a chart saved before that change gives
```
TypeError: pivot() got an unexpected keyword argument 'flatten_columns'
```
on every one of those paths, while the same chart renders correctly in
Explore. There is no migration for the stored query, so the failure is
permanent until somebody opens each chart and re-saves it.
**The change.** `QueryObject` compares the stored options against the
signature of the operation and drops those it no longer accepts, logging a
warning that names the operation and the options. Comparing against the
signature avoids a hard-coded list of removed names, which would need extending
at each release. An operation that takes `**kwargs` is left alone. An unknown
operation is left in place so that `exec_post_processing` still reports it as
`InvalidPostProcessingError`.
**Why `functools.wraps` is in the same PR.** That comparison needs a
signature to read, and there was none. `validate_column_args` returned `def
wrapped(df, **options)` without `wraps`:
```
>>> inspect.signature(pivot)
(df: object, **options: object) -> object
>>> pivot.__name__
'wrapped'
```
All ten operations using that decorator (`aggregate`, `compare`,
`contribution`, `cum`, `diff`, `pivot`, `rename`, `rolling`, `select`, `sort`)
reported `**kwargs` and lost their name and docstring. `inspect.unwrap` cannot
recover the original, because without `wraps` there is no `__wrapped__`. Adding
`wraps` restores the signature, the name and the docstring. Happy to split this
into its own PR if you prefer.
**Scope.** The second symptom in #42926 — a stored query that sets
`is_timeseries` without a temporal column — is deliberately not addressed here.
`_apply_granularity` has since gained its own inference path, so the intended
behaviour there deserves a maintainer's opinion before I send code.
**Behaviour for current charts is unchanged.** A `query_context` built by
the current frontend has options that match the current signature, so nothing
is dropped and the same dict object is returned.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Server-side; no UI change.
Before, on a chart saved in 2023:
```
GET /api/v1/chart/6132/data/
500 TypeError: pivot() got an unexpected keyword argument 'flatten_columns'
```
After, the same chart returns its rows. The chart itself was never broken in
Explore, before or after.
### TESTING INSTRUCTIONS
New unit tests:
- `tests/unit_tests/queries/query_object_test.py`
- a stored `pivot` with `flatten_columns` and `reset_index` keeps only the
supported options, and keeps their values
- a current `query_context` is returned unchanged, as the same object
- an unknown operation is preserved for `exec_post_processing` to reject
- `tests/unit_tests/pandas_postprocessing/test_utils.py`
- the decorator exposes the signature and the name of the operation it
wraps
Manual check on any version:
```python
import inspect
from superset.utils.pandas_postprocessing import pivot
print(inspect.signature(pivot)) # before: (df, **options); after: the real
parameters
```
This change is also running in production on a 6.1.0 instance, applied as a
patch. Three charts that had failed for years — an `echarts_timeseries_bar` and
a `big_number` saved in 2023 — now return data through the chart-data endpoint,
and charts saved recently are unaffected.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42926
- [ ] 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]
