SEPURI-SAI-KRISHNA opened a new issue, #45017:
URL: https://github.com/apache/superset/issues/45017
### Bug description
`_append_columns` decides between overwriting and appending with `all()`, so
a mapping that does both falls entirely to the append path:
```python
# superset/utils/pandas_postprocessing/utils.py
if all(key == value for key, value in columns.items()):
_base_df = base_df.copy()
_base_df.loc[:, columns.keys()] = append_df
return _base_df
append_df = append_df.rename(columns=columns)
return pd.concat([base_df, append_df], axis="columns")
```
The column that should have been overwritten is re-appended under its own
name, so the result carries two columns with the same label. Every operator
taking a `columns: dict[str, str]` mapping is affected: `cum`, `rolling`,
`diff`, `geohash_decode`, `geohash_encode`, `geodetic_parse`.
Measured on master (`df` has columns `y` and `z`):
| mapping | result | |
|---|---|---|
| `{"y": "y", "z": "z"}` | `['y', 'z']` | ✅ all overwrite |
| `{"y": "y2", "z": "z2"}` | `['y', 'z', 'y2', 'z2']` | ✅ all rename |
| `{"y": "y", "z": "z2"}` | `['y', 'z', 'y', 'z2']` | ❌ `y` duplicated |
```python
import pandas as pd
from superset.utils.pandas_postprocessing import cum
df = pd.DataFrame({"y": [1.0, 2.0, 3.0], "z": [4.0, 5.0, 6.0]})
out = cum(df=df, operator="sum", columns={"y": "y", "z": "z2"})
out.columns.tolist() # ['y', 'z', 'y', 'z2']
type(out["y"]) # <class 'pandas.DataFrame'> -- callers
expect a Series
```
`rolling` and `diff` reproduce identically. It also happens with MultiIndex
columns, which is the realistic chart path. Cumulating one metric in place
after a pivot while adding another alongside it duplicates `('sum_metric',
'UK')` and `('sum_metric', 'US')`.
Downstream, `df["y"]` returning a 2-column DataFrame instead of a Series
breaks `flatten`, serialization, and any later post-processing step.
### A second defect in the same branch
The append path concatenates the whole of `append_df`, including columns the
mapping never asked for. `geodetic_parse` always parses an altitude but only
maps it when the caller requested one:
```python
post_df = geodetic_parse(
df=lonlat_df[["city", "geodetic"]],
geodetic="geodetic",
latitude="lat",
longitude="lon", # no altitude requested
)
post_df.columns.tolist() # ['city', 'geodetic', 'lat', 'lon', 'altitude']
```
An unrequested `altitude` column reaches the result. The all-overwrite path
does *not* leak it, because `.loc[:, keys] = append_df` aligns by label and
ignores the extra column, so the two paths already disagree. The existing
`test_geodetic_parse` covers only the path that behaves correctly.
`geohash_encode` is worse, because the columns it leaks collide with the
ones already in the frame. It builds its working frame with hardcoded
`latitude`/`longitude` labels and maps only the geohash, so naming the geohash
column anything other than `geohash` makes the mapping a pure rename and sends
the whole working frame through the concat:
```python
out = geohash_encode(
df=lonlat_df[["city", "latitude", "longitude"]],
latitude="latitude",
longitude="longitude",
geohash="hash", # any name but "geohash" takes the rename path
)
out.columns.tolist() # ['city', 'latitude', 'longitude',
'latitude', 'longitude', 'hash']
out.columns.duplicated().any() # True
type(out["latitude"]) # <class 'pandas.DataFrame'>
```
So the duplicate-label failure is not confined to mixed mappings: an
all-rename mapping reaches it too whenever `append_df` carries a column that
`base_df` already has. Both defects come from the same two lines, and selecting
before renaming fixes both.
### How to reproduce the bug
1. Send a chart-data request whose `post_processing` passes a `columns`
mapping where some entries rename and others do not, e.g. `{"operation": "cum",
"options": {"operator": "sum", "columns": {"y": "y", "z": "z2"}}}`.
2. The response carries two columns labelled `y`.
### Why it was not caught
No test in `test_cum.py`, `test_rolling.py` or `test_diff.py` passes a mixed
mapping. Every case is either all-overwrite or all-rename, where `all()` and
`any()` agree. The geography tests miss their half for a different reason:
`test_geohash_encode` and `test_geodetic_parse` name the target columns exactly
as the working frame names them, which is an identity mapping, so both take the
overwrite path and never exercise the concat.
### Suggested fix
Split the mapping and handle each half, selecting before renaming so
unmapped columns are dropped:
```python
overwritten = {key: value for key, value in columns.items() if key == value}
appended = {key: value for key, value in columns.items() if key != value}
```
### Screenshots/recordings
_No response_
### Superset version
master / latest-dev
### Python version
3.11
### Node version
Not applicable
### Browser
Not applicable
### Additional context
_No response_
### Checklist
- [x] I have searched Superset docs and Slack and didn't find a solution to
my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar bug
report.
- [x] I have checked Superset's logs for errors and if I found a relevant
Python stacktrace, I included it here as text in the "additional context"
section.
--
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]
