Copilot commented on code in PR #40859:
URL: https://github.com/apache/superset/pull/40859#discussion_r3375256376
##########
superset/charts/client_processing.py:
##########
@@ -388,9 +389,10 @@ def apply_client_processing( # noqa: C901
if query["result_format"] == ChartDataResultFormat.JSON:
query["data"] = processed_df.to_dict()
elif query["result_format"] == ChartDataResultFormat.CSV:
- buf = StringIO()
- processed_df.to_csv(buf, index=show_default_index)
- buf.seek(0)
- query["data"] = buf.getvalue()
+ # Route through the formula-escaping CSV writer, consistent with
the
+ # other CSV export paths (viz, query context, SQL Lab export).
+ query["data"] = csv.df_to_escaped_csv(
+ processed_df, index=show_default_index
+ )
Review Comment:
`csv.df_to_escaped_csv()` mutates object-dtype columns using
`enumerate(column.values)` + `df.at[idx, name]`, which is label-based. If
`processed_df` has a non-RangeIndex (common for `pivot_table_v2`, e.g. index
label "Total (Sum)"), this can raise `KeyError` as soon as there’s any string
cell to escape (including formula-trigger values). This new call site makes
that failure reachable for post-processed CSV exports.
Fix should be in `superset/utils/csv.py:df_to_escaped_csv()` to iterate with
index labels (e.g. `for idx, value in column.items(): ...`) or use positional
`.iat` safely, so it works for arbitrary indexes.
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -2152,6 +2152,52 @@ def
test_apply_client_processing_csv_format_simple_table():
}
+def test_apply_client_processing_csv_format_escapes_formula_values():
+ """
+ A value starting with a formula trigger should be escaped in the CSV
+ output, consistent with the other CSV export paths.
+ """
Review Comment:
This test covers the `viz_type="table"` path where the DataFrame index
remains 0..N, but it doesn’t cover the `pivot_table_v2`/post-processed CSV path
where `apply_client_processing()` produces a non-RangeIndex (e.g. "Total
(Sum)") and then serializes via `df_to_escaped_csv()`. Adding a regression test
for `viz_type="pivot_table_v2"` with a string CSV value starting with `=` would
catch index-related escaping failures and ensure formula escaping works on the
main post-processed export use case.
--
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]
