sadpandajoe opened a new pull request, #43720:
URL: https://github.com/apache/superset/pull/43720
### SUMMARY
Legacy Pivot Table charts on BigQuery connections error when a Custom SQL
row is labeled with a
name containing a space (e.g. `Test Row`), once a metric is added.
`get_sqla_query()` built the `GROUPING SETS` level lookup and the
`GROUPING()` marker column labels
keyed by each column's engine-mutated SQL alias (`Label.name`) instead of
its logical/requested
label (`Label.key`). BigQuery's label mutation rewrites labels containing
non-word characters (e.g.
a space) into a different string for `.name`, while `.key` preserves the
original label. The
frontend's `grouping_sets` levels and `GROUPING()` marker lookup are
expressed in terms of the
original label, so keying by the mutated `.name` silently dropped the column
from every rollup
level, producing an invalid `GROUP BY GROUPING SETS` clause missing a
selected, non-aggregated
column.
The fix keys the lookup by each column's `.key` instead, and routes the
`GROUPING()` marker column
through `make_sqla_column_compatible` like every other selected column so
the SQL-level alias is
still engine-mutated when required while the logical label used for
downstream matching stays
unmutated.
### BEFORE/AFTER
Before: Pivot Table on a BigQuery connection with a Custom SQL row labeled
`Test Row` and any
metric throws an error when rendering.
After: chart renders correctly; the `GROUPING SETS` clause includes the
custom-SQL column at every
expected rollup level.
### TESTING INSTRUCTIONS
1. Connect a BigQuery database.
2. Create a legacy Pivot Table chart (not the AG Grid / Interactive Pivot
Table) on a BigQuery
dataset.
3. Add a Row using Custom SQL, labeled with a name containing a space (e.g.
`Test Row`).
4. Add any metric.
5. Confirm the chart renders without error and the rollup includes the
custom-SQL row.
Also covered by a new unit test,
`tests/unit_tests/models/helpers_test.py::test_get_sqla_query_grouping_sets_preserves_bigquery_mutated_label`,
which reproduces the bug against a mocked BigQuery dialect and asserts the
fixed `GROUPING SETS`
clause.
### ADDITIONAL INFORMATION
- [x] Has associated tests
- [ ] 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]
