SEPURI-SAI-KRISHNA opened a new issue, #45042:
URL: https://github.com/apache/superset/issues/45042
### Bug description
`rename`'s guard against renaming a column onto a label that already exists
uses `all` where it needs `any`, so it only fires when *every* new name
collides. A mapping where some names collide and some do not slips past it, and
pandas then creates a second column under the colliding label.
```python
# superset/utils/pandas_postprocessing/rename.py:52
if all(new_name in _rename_level for new_name in columns.values()):
raise InvalidPostProcessingError(_("Label already exists"))
```
With a frame whose columns are `[a, b, c]`:
| mapping | `all(...)` | `any(...)` | result |
|---|---|---|---|
| `{"a": "b"}` | True | True | raises `Label already exists` |
| `{"a": "b", "c": "z"}` | False | True | `['b', 'b', 'z']`, no error |
The second row is the bug. `b` appears twice, `df["b"]` returns a DataFrame
where every caller expects a Series, and the duplicate travels on to whatever
post-processing operation runs next.
### How to reproduce the bug
```python
import pandas as pd
from superset.utils.pandas_postprocessing import rename
df = pd.DataFrame({"a": [1, 2], "b": [3, 4], "c": [5, 6]})
rename(df=df, columns={"a": "b"})
# InvalidPostProcessingError: Label already exists
out = rename(df=df, columns={"a": "b", "c": "z"})
print(out.columns.tolist()) # ['b', 'b', 'z']
print(out.columns.duplicated().any()) # True
print(type(out["b"]).__name__) # DataFrame
```
The same holds on a MultiIndex with `level=0`.
### Why it was not caught
Both duplication tests in
`tests/unit_tests/pandas_postprocessing/test_rename.py`,
`test_should_raise_exception_duplication` and
`test_should_raise_exception_duplication_on_multiindex`, pass a single-entry
mapping. For one entry `all` and `any` are the same condition, so the inversion
is invisible. The three tests that do pass a two-entry mapping rename onto
names that are free, so they never reach the guard.
### Suggested fix
Change `all` to `any`. The existing message already states the intent, and
no existing test changes behaviour: a mapping whose targets are all free still
renames, and a mapping with any colliding target now raises as the single-entry
case already did.
### 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]
