bito-code-review[bot] commented on PR #43222:
URL: https://github.com/apache/superset/pull/43222#issuecomment-5302589258
<!-- Bito Reply -->
The flagged issue is correct. When `Decimal` values are included in the
query context and subsequently hashed for caching, the `json_int_dttm_ser`
serializer converts them to `float`. This lossy conversion can cause distinct
`Decimal` totals to hash to the same cache key, leading to incorrect cached
contribution percentages.
To resolve this, you should ensure `Decimal` values are serialized to
strings or another high-precision format during cache key generation, or
exclude these runtime totals from the cache key entirely. A common approach is
to update the serializer to handle `Decimal` types explicitly by converting
them to strings, which preserves precision.
There are no other comments on this PR to address.
**superset/common/query_context_processor.py**
```
def _is_summable(series: pd.Series) -> bool:
"""
Whether a column holds values the contribution totals can be summed over.
A dtype-kind test alone is not enough. `decimal.Decimal` metrics -- how
drivers such as psycopg2 hand back NUMERIC/DECIMAL columns -- are stored
with an object dtype, so they fall outside the numeric kinds even though
they sum and divide perfectly well. Omitting them leaves the totals
dictionary without an entry for the metric, and `contribution()` then
reads back `None` and writes a zero contribution instead of the real
percentage.
"""
if series.dtype.kind in "biufc":
return True
return infer_dtype(series, skipna=True) == "decimal"
```
--
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]
