daviddallakyan2005 opened a new pull request, #43219:
URL: https://github.com/apache/superset/pull/43219
### SUMMARY
Follow-up to #43218 (empty query crash). This change does not include that
guard.
Histogram `transformProps` builds the x-axis from every datum key that is
not in `groupby`, then parses those leftover keys as `"low - high"` bin edges.
Series names already look up groupby values with `getColumnLabel()`, but
membership was checked against the raw form-data entries:
```ts
const groupbySet = new Set(groupby);
```
Adhoc groupby columns are objects (`{ sqlExpression, label, expressionType:
'SQL' }`), so `Set.has(datumKey)` is always false. The labeled column is
treated as a bin, `parseFloat(label)` is `NaN`, and the x-axis grows an extra
`"NaN - undefined"` category (or `"NaN - NaN"` with a numeric formatter). The
series data array also picks up the groupby value.
Sibling plugins (BoxPlot, Radar, Pie) already map
`groupby.map(getColumnLabel)` before comparing. This does the same, so physical
string columns stay identity and adhoc labels match the keys the query actually
returns.
### TESTING INSTRUCTIONS
Automated:
```bash
cd superset-frontend
npm run test -- plugins/plugin-chart-echarts/test/Histogram # 6 passed
npm run test -- plugins/plugin-chart-echarts # 85 suites,
896 passed
```
Reverting the one-line change makes the new adhoc test fail:
```
● does not treat adhoc groupby labels as histogram bin edges
Expected length: 2
Received length: 3
Received array: ["NaN - undefined", "0 - 5", "5 - 10"]
```
Manually:
1. Create a Histogram chart with a numeric column and an adhoc groupby (SQL
expression with a custom label).
2. Before: the x-axis includes a NaN bin and the groupby value is mixed into
the series data.
3. After: the x-axis is only the numeric bins, and the series is named from
the groupby value.
### 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
Made with [Cursor](https://cursor.com)
--
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]
