codeant-ai-for-open-source[bot] commented on code in PR #42761:
URL: https://github.com/apache/superset/pull/42761#discussion_r3716451119
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -970,32 +980,53 @@ class PivotData {
);
const vals = this.props.vals as string[];
+ const fractionType =
+ FRACTION_TYPE_BY_SHOW_VALUES_AS[this.props.showValuesAs as string];
// Values come pre-aggregated from the database (one query per rollup
level),
// so the pivot stores them verbatim via `cellValue` instead of
aggregating.
- this.aggregator = cellValue(this.props.defaultFormatter as
Formatter)(vals);
- this.formattedAggregators = this.props.customFormatters
- ? Object.entries(
- this.props.customFormatters as Record<
- string,
- Record<string, unknown>
- >,
- ).reduce(
- (
- acc: Record<
+ // When "Show values as" a fraction is active, wrap that passthrough with
+ // the same fractionOf template the pre-SIP-216 "Sum as Fraction of ..."
+ // aggregators used: it divides a cell's own value by the value at the
+ // requested scope (row/column/grand total), each of which is itself one
+ // of these uniformly-wrapped aggregators, so `fractionOf`'s cross-lookup
+ // (`data.getAggregator(...).inner.value()`) resolves correctly no matter
+ // which scope it's called for -- including the totals dividing by
+ // themselves to read 100%. This needs no new query and no per-metric
+ // aggregator-override control (that control is gone, see SIP.md); it's a
+ // pure display transform over values that are already DB-correct.
+ this.aggregator = fractionType
+ ? aggregatorTemplates.fractionOf(
+ cellValue(),
+ fractionType,
+ usFmtPct,
+ )(vals)
+ : cellValue(this.props.defaultFormatter as Formatter)(vals);
Review Comment:
**Suggestion:** When `percent_row` or `percent_col` is selected while the
corresponding total toggle is disabled, `buildGroupbyCombinations` does not
load the denominator level. `getAggregator` then returns its null placeholder,
and `fractionOf` dereferences `.inner.value()` on that placeholder, causing
rendering to fail. Fraction display must request the required rollup levels
independently of whether those totals are visible, or handle missing
denominators safely. [null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Pivot rendering fails for row percentages without row totals.
- ❌ Column percentages fail when column totals are disabled.
- ⚠️ Total percentages require both hidden rollup levels.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e1094ed8c79d406aa16bc866c3dec238&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e1094ed8c79d406aa16bc866c3dec238&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts
**Line:** 997:1003
**Comment:**
*Null Pointer: When `percent_row` or `percent_col` is selected while
the corresponding total toggle is disabled, `buildGroupbyCombinations` does not
load the denominator level. `getAggregator` then returns its null placeholder,
and `fractionOf` dereferences `.inner.value()` on that placeholder, causing
rendering to fail. Fraction display must request the required rollup levels
independently of whether those totals are visible, or handle missing
denominators safely.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42761&comment_hash=c000389dd483c2564c455ce9de1e281d9c85b61bc824faac0698f2d5f0a04337&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42761&comment_hash=c000389dd483c2564c455ce9de1e281d9c85b61bc824faac0698f2d5f0a04337&reaction=dislike'>👎</a>
--
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]
