sadpandajoe opened a new pull request, #41401:
URL: https://github.com/apache/superset/pull/41401
### SUMMARY
In a **Mixed Chart** (`mixed_timeseries`), changing **"Sort Query By"** or
toggling **"Sort Descending"** updated the displayed SQL ("View query") but did
**not** change the rendered result.
Root cause is in the frontend query builder. `MixedTimeseries/buildQuery.ts`
returned the full output of `normalizeOrderBy(tmpQueryObject)`.
`normalizeOrderBy` deletes `order_desc` and `series_limit_metric` from the
object it returns, re-adding only `orderby`. The backend main query reads
`orderby` (so the displayed SQL looks correct), but the **series-limit
subquery** that decides *which* top-N series to show reads `order_desc` and
`series_limit_metric` directly (`superset/models/helpers.py`: `direction =
sa.desc if order_desc else sa.asc`, and `_get_series_orderby` reads
`series_limit_metric`). With those two fields stripped, the backend
`QueryObject` defaults `order_desc` to `True`, so the same top-N series were
always selected → the result never changed.
The fix mirrors what the single-query **Timeseries** chart already does:
spread the query object and override only `orderby`, preserving `order_desc`
and `series_limit_metric`.
```js
// MixedTimeseries/buildQuery.ts
return [
{
...tmpQueryObject,
orderby: normalizeOrderBy(tmpQueryObject).orderby,
},
];
```
This applies symmetrically to Query A and Query B.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no visual chrome change. The behavior change is that the rendered
series now respond to the sort controls (see Testing Instructions).
### TESTING INSTRUCTIONS
1. Create a Mixed Chart (Mixed Timeseries) on a dataset with a dimension
that produces more series than the series limit (e.g. group by a
high-cardinality column with **Series limit** set to a small N).
2. Set **"Sort Query By"** to a metric and toggle **"Sort Descending"** on,
then off.
3. **Before this fix:** the displayed SQL changes but the rendered top-N
series stay the same.
4. **After this fix:** the rendered top-N series change to honor the chosen
sort metric and direction, for both Query A and Query B.
Automated coverage
(`plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts`):
- Updated "should compile query object A/B" to assert `order_desc` and
`series_limit_metric` are preserved.
- Added a regression test, `preserves order_desc and series_limit_metric for
both queries`, asserting the sc-107146 ascending-sort repro keeps `order_desc:
false` + `series_limit_metric` for both queries. Reverting only the source fix
fails this test.
Run: `npm run test --
plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts`
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] 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
#### Reviewer / QA note — expected behavior change on existing saved charts
Preserving `series_limit_metric` changes the series-limit subquery's ORDER
BY **column**, not just its direction. Previously the field was stripped, so
the backend fell back to ordering the top-N prequery by the main metric; with
the fix it orders by the chosen **"Sort Query By"** metric (`helpers.py`
`_get_series_orderby`). This is the intended correctness fix, but **existing
saved Mixed Charts may render a different (correct) top-N series set with no
user action**. Please don't mistake this for a regression during QA.
--
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]
