SEPURI-SAI-KRISHNA opened a new pull request, #43222:
URL: https://github.com/apache/superset/pull/43222
<!-- PR TITLE: fix(chart): include Decimal metrics in contribution totals -->
### SUMMARY
A `Decimal` metric renders as **0%** in every row of a contribution chart
that
uses cross-query totals — no error, just silently wrong numbers.
`ensure_totals_available()` builds the totals dictionary handed to the
`contribution` post-processing operation:
```python
totals = {
col: df[col].sum() for col in df.columns if df[col].dtype.kind in "biufc"
}
```
`dtype.kind in "biufc"` covers bool, signed/unsigned int, float and complex.
It does not cover `object`, which is where `decimal.Decimal` values live —
that is how drivers such as psycopg2 hand back `NUMERIC`/`DECIMAL` columns.
So a Decimal metric never gets a totals entry, and `contribution()` takes the
zero branch:
```python
total = contribution_totals.get(col)
if total is None or total == 0:
contribution_df[rename_col] = 0
```
The values themselves are perfectly summable and divisible — `Series.sum()`
on
a Decimal column returns a `Decimal`, and `Decimal / Decimal` is exact. Only
the dtype test excludes them.
This PR replaces the inline dtype check with `_is_summable()`, which keeps
the
numeric-kind test and additionally accepts object columns whose inferred
value
type is `decimal`. Strings, dicts, datetimes and all-null object columns stay
excluded exactly as before.
Found while reviewing #43203, which fixes a different symptom of the same
"Decimal lives in an object column" root cause in `contribution()` itself.
The
two are independent and touch different files.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only fix.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/common/test_query_context_processor.py
```
Three tests are added:
| test | asserts |
| --- | --- |
| `test_is_summable_accepts_decimal_columns` | Decimal (with and without
nulls) is summable; strings/dicts/datetimes are not — fails on `master` |
| `test_ensure_totals_available_includes_decimal_metrics` | a Decimal metric
reaches `contribution_totals`, a string column does not — fails on `master` |
| `test_contribution_uses_decimal_totals_rather_than_zero` | end to end:
real percentages with the totals present, zeros without |
The third passes either way by design — it pins the downstream consequence
using explicit totals dicts rather than the builder, documenting exactly what
the missing entry costs.
Manually: on Postgres, build a chart over a `NUMERIC` metric with
Contribution
Mode enabled on a viz that computes cross-query totals. On `master` the
contribution column is 0 for every row; with this change it shows the real
percentages.
### 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
### CHECKLIST
- [ ] CI checks pass
- [x] Tests added/updated
- [ ] Documentation updated
- [x] PR title follows conventions
--
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]
