You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org> on 2026/10/07 05:12:23 UTC

[I] A post-processing `rename` mapping that partially collides creates a duplicate column label [superset]

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: notifications-unsubscribe@superset.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] A post-processing `rename` mapping that partially collides creates a duplicate column label [superset]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.
schizophrenicmaniac commented on issue #45042:
URL: https://github.com/apache/superset/issues/45042#issuecomment-6031502792

   Hi, I'd like to work on this. The guard in rename() in pandas_postprocessing/rename.py should use any() instead of all(). I'd make that change and add tests to test_rename.py for a partly colliding mapping, on both a flat index and a MultiIndex.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] A post-processing `rename` mapping that partially collides creates a duplicate column label [superset]

Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on issue #45042:
URL: https://github.com/apache/superset/issues/45042#issuecomment-6036088198

   Correcting the suggested fix above, which is not right and I should not have stated it that confidently.
   
   `all` to `any` rejects a mapping that renames onto a label it vacates itself. `{"a": "b", "b": "d"}` on `[a, b, c]` gives `['b', 'd', 'c']`, which is unique, and master accepts it today. Flagged by CodeAnt on #45043.
   
   There is also a second form of the same bug that neither `all` nor `any` catches: `{"a": "z", "b": "z"}` gives `['z', 'z', 'c']`, with no target present beforehand for a presence test to find.
   
   The guard is asking whether a target label exists when the thing that matters is whether the result holds a duplicate. #45043 checks that instead.
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] A post-processing `rename` mapping that partially collides creates a duplicate column label [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed issue #45042: A post-processing `rename` mapping that partially collides creates a duplicate column label
URL: https://github.com/apache/superset/issues/45042


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] A post-processing `rename` mapping that partially collides creates a duplicate column label [superset]

Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on issue #45042:
URL: https://github.com/apache/superset/issues/45042#issuecomment-6031561753

   Thanks for offering. A PR for this is already open: #45043. It was pushed before this comment landed, so the issue went up with the fix ready rather than as up-for-grabs work. My fault for not saying that in the description.
   
   It does what you described, `any` instead of `all`, with a test for a partly colliding mapping on a flat index and one on a MultiIndex at `level=0`.
   
   Review on the PR is welcome if you want a look.
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org