Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,9 @@ export function TableRenderer(props: TableRendererProps) {
true,
)}
>
{t('Subtotal')}
{t('Subvalue (%(aggregatorName)s)', {
aggregatorName: t(aggregatorName),
})}
</th>
) : null;

Expand Down
4 changes: 2 additions & 2 deletions superset/charts/client_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
slice_ = df.columns.get_loc(subgroup)
subtotal = pivot_v2_aggfunc_map[aggfunc](df.iloc[:, slice_], axis=1)
depth = df.columns.nlevels - len(subgroup) - 1
total = metric_name if level == 0 else __("Subtotal")
total = metric_name if level == 0 else __("Subvalue (%(aggfunc)s)", aggfunc=aggfunc)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This new subtotal label diverges from the frontend pivot-table renderer, which still renders subgroup headers as Subtotal. Since this module is meant to reproduce client-side processing for reports, changing only the backend label creates a contract mismatch (Explore UI vs CSV/JSON/report output labels differ). Keep both sides aligned by updating the frontend subtotal label logic in the same change or using the same label here. [api mismatch]

Severity Level: Major ⚠️
- ⚠️ Pivot table CSV subtotals differ from Explore UI.
- ⚠️ Scheduled pivot_table_v2 reports show renamed subtotal labels.
- ⚠️ Downstream tooling relying on UI labels may misalign.
Steps of Reproduction ✅
1. Note that `superset/charts/client_processing.py:18-25` documents this module as
reproducing client-side post-processing so that reports show the same data as Explore
(pivot table UI).

2. On the frontend, `pivot_table_v2` uses the pivot-table plugin wired via
`transformProps` at
`superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts:98-120`
and `PivotTableChart` (see plugin wiring); it passes subtotal controls
(`rowSubtotalPosition`, `colSubtotalPosition`, etc.) but does not receive or override the
backend subtotal label, so the UI continues to use the pivot library's built‑in subtotal
labeling (currently "Subtotal").

3. For reports/exports, the backend entrypoint `apply_client_processing()` at
`superset/charts/client_processing.py:13-27` routes `viz_type == "pivot_table_v2"` through
`pivot_table_v2()`, which in turn calls `pivot_df()` with
`aggfunc=form_data["aggregateFunction"]` to recompute the pivot for CSV/JSON (see tests
`test_apply_client_processing_json_format` and `test_apply_client_processing_csv_format`
at `tests/unit_tests/charts/test_client_processing.py:1193-1379`).

4. Inside `pivot_df()`, subtotals are now labeled with `__("Subvalue (%(aggfunc)s)",
aggfunc=aggfunc)` for non-top levels (`superset/charts/client_processing.py:77` for
`metric_name` and lines `119` and `26` for `total`), so for `aggfunc="Sum"` the exported
CSV/JSON indices contain "Subvalue (Sum)" while the Explore UI subtotal headers remain
"Subtotal"; this breaks the intended parity between Explore and exported reports for
subtotal labels.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset/charts/client_processing.py
**Line:** 178:178
**Comment:**
	*Api Mismatch: This new subtotal label diverges from the frontend pivot-table renderer, which still renders subgroup headers as `Subtotal`. Since this module is meant to reproduce client-side processing for reports, changing only the backend label creates a contract mismatch (Explore UI vs CSV/JSON/report output labels differ). Keep both sides aligned by updating the frontend subtotal label logic in the same change or using the same label here.

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
👍 | 👎

subtotal_name = tuple([*subgroup, total, *([""] * depth)]) # noqa: C409
# insert column after subgroup
df.insert(int(slice_.stop), subtotal_name, subtotal)
Expand All @@ -201,7 +201,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
df.iloc[slice_, :].apply(pd.to_numeric, errors="coerce"), axis=0
)
depth = groups.nlevels - len(subgroup) - 1
total = metric_name if level == 0 else __("Subtotal")
total = metric_name if level == 0 else __("Subvalue (%(aggfunc)s)", aggfunc=aggfunc)
subtotal.name = tuple([*subgroup, total, *([""] * depth)]) # noqa: C409
# insert row after subgroup
df = pd.concat(
Expand Down
13 changes: 11 additions & 2 deletions superset/commands/streaming_export/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
@contextmanager
def preserve_g_context(
captured_g: dict[str, Any],
user: Any | None = None,
) -> Generator[None, None, None]:
"""
Context manager that restores captured flask.g attributes.
Expand All @@ -47,9 +48,13 @@ def preserve_g_context(

Args:
captured_g: Dictionary of g attributes captured before context switch
user: Optional user object to set as g.user (for impersonation)
"""
for key, value in captured_g.items():
setattr(g, key, value)
# Explicitly set g.user for user impersonation to work
if user is not None:
g.user = user
yield


Expand Down Expand Up @@ -169,9 +174,11 @@ def _execute_query_and_stream(
catalog=catalog, schema=schema
) as engine:
with engine.connect() as connection:
# Apply config-based SQL mutations (e.g., remove trailing semicolons)
mutated_sql = merged_database.mutate_sql_based_on_config(sql)
result_proxy = connection.execution_options(
stream_results=True
).execute(text(sql))
).execute(text(mutated_sql))

columns = list(result_proxy.keys())

Expand Down Expand Up @@ -222,11 +229,13 @@ def run(self) -> Callable[[], Generator[str, None, None]]:
captured_g = (
g._get_current_object().__dict__.copy() if has_app_context() else {}
)
# Capture user separately for impersonation - g.user may not serialize properly
user = getattr(g, "user", None) if has_app_context() else None

def csv_generator() -> Generator[str, None, None]:
"""Generator that yields CSV data chunks."""
with self._current_app.app_context():
with preserve_g_context(captured_g):
with preserve_g_context(captured_g, user=user):
try:
yield from self._execute_query_and_stream(
sql, database, limit, catalog, schema
Expand Down
3 changes: 2 additions & 1 deletion superset/translations/en/LC_MESSAGES/messages.po
Original file line number Diff line number Diff line change
Expand Up @@ -11095,7 +11095,8 @@ msgstr ""
msgid "Subtitle"
msgstr ""

msgid "Subtotal"
#, python-format
msgid "Subvalue (%(aggregatorName)s)"
msgstr ""

msgid "Success"
Expand Down
Loading