SEPURI-SAI-KRISHNA opened a new issue, #45082:
URL: https://github.com/apache/superset/issues/45082
### Bug description
`_append_columns` splits a `columns` mapping into an overwrite half and an
append half by asking whether the source and target names match. A rename can
land on a label that is already in the frame, and that case goes down the
append path, so the result carries two columns under the same name.
#45018 fixed the mixed-mapping case of this (#45017) by introducing that
split. This is the residual: @EnxDev raised it in review on #45018 and I
recorded it on #45017, which auto-closed as completed when #45018 merged, so it
is filed here on its own.
Reproduction on current master:
```python
from pandas import DataFrame
from superset.utils.pandas_postprocessing.utils import _append_columns
base_df = DataFrame({"y": [1, 2], "z": [3, 4]})
append_df = DataFrame({"y": [10, 20]})
_append_columns(base_df, append_df, {"y": "z"})
# y z z
# 0 1 3 10
# 1 2 4 20
```
`base_df` already has a `z`, so the mapping renames onto an existing column.
Because `y != z`, the pair is appended instead of overwriting, and
`columns.duplicated().any()` is true.
Expected: the mapping overwrites `z`, giving `['y', 'z']` with `z` holding
`[10, 20]`.
A duplicate label makes `df["z"]` return a DataFrame where every caller
expects a Series, which is the same failure mode as #45017: downstream
operations that do a per-column operation raise, and the chart data response
carries two fields with the same name.
Reached through any operation that passes a user-supplied mapping to
`_append_columns`, which is `cum`, `diff` and `rolling`, plus `geohash_decode`,
`geohash_encode` and `geodetic_parse` where the new column is named after one
that already exists.
### How to reproduce the bug
1. Create a chart whose query result has columns `y` and `z`.
2. Add a `cum` post-processing operation with `columns: {"y": "z"}`.
3. The response carries two `z` columns.
### Screenshots/recordings
_No response_
### Superset version
master / latest-dev
### Python version
3.11
### Node version
Not applicable
### Browser
Not applicable
### Additional context
On #45018 I suggested keying the split on `value in base_df.columns` and
said it subsumes the matching case, "since a `{"y": "y"}` target is present by
definition". That is wrong, and I am correcting it here before anyone builds on
it.
It holds for `cum`, `diff` and `rolling`, where `@validate_column_args`
guarantees the mapping's keys are columns of `base_df`. It does not hold for
the geography operations, where the keys are `append_df`'s column names:
`geodetic_parse` passes `{"latitude": "latitude"}` whenever the caller keeps
the default column name, and `latitude` is not in `base_df` at all. Keying on
presence alone would send that down the append path, and `pd.concat` unions the
index rather than aligning, so a frame without a 0-based index gains a row per
parsed row:
```python
base_df = DataFrame({"city": ["A", "B"]}, index=[5, 6])
append_df = DataFrame({"latitude": [41.1, 40.7]})
# presence-keyed split: 4 rows instead of 2
```
Both halves of the condition are needed. An entry should be appended only
when its target is a label the result does not already have, meaning the target
differs from the source **and** is not already in `base_df`. Measured over 1152
mappings across four index shapes, that condition changes behaviour only where
a target already exists, with no column reordering and no case where the row
count moves except where the old code was producing both a duplicate label and
extra rows.
### 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 or in a screenshot.
--
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]
