aminghadersohi commented on code in PR #44189:
URL: https://github.com/apache/superset/pull/44189#discussion_r4009435809
##########
tests/unit_tests/result_set_test.py:
##########
@@ -21,19 +21,53 @@
import numpy as np
import pandas as pd
+import pytest
from numpy.core.multiarray import array
from pytest_mock import MockerFixture
from superset.db_engine_specs.base import BaseEngineSpec
from superset.result_set import (
+ dedup,
stringify_extension_columns,
stringify_values,
SupersetResultSet,
)
-from superset.superset_typing import DbapiResult
+from superset.superset_typing import DbapiDescription, DbapiResult
from superset.utils import json as superset_json
[email protected](
+ "names,case_sensitive,expected",
+ [
+ (["a", "a", "a__1"], True, ["a", "a__2", "a__1"]),
+ (["a__1", "a", "a"], True, ["a__1", "a", "a__2"]),
+ (["a", "a", "a__1", "a__2"], True, ["a", "a__3", "a__1", "a__2"]),
+ (["a", "A", "A__1"], False, ["a", "A__2", "A__1"]),
+ (["a", "A", "A__1"], True, ["a", "A", "A__1"]),
+ ],
+)
+def test_dedup_preserves_explicit_column_names(
+ names: list[str], case_sensitive: bool, expected: list[str]
+) -> None:
+ assert dedup(names, case_sensitive=case_sensitive) == expected
+
+
+def test_result_set_with_colliding_column_suffix() -> None:
+ description: DbapiDescription = [
+ (name, "INT", None, None, None, None, False) for name in ("a", "a",
"a__1")
+ ]
+ result = SupersetResultSet([(1, 2, 3)], description, BaseEngineSpec)
+ assert result.to_pandas_df().to_dict("records") == [{"a": 1, "a__2": 2,
"a__1": 3}]
+
+
+def test_dedup_case_insensitive_custom_suffix() -> None:
+ assert dedup(["a", "A", "ax1"], suffix="X", case_sensitive=False) == [
+ "a",
+ "AX2",
+ "ax1",
+ ]
Review Comment:
`reserved.add(...)` is the one conjunct no test pins — deleting it leaves
all 27 green. It only bites when two different bases generate the same name,
which the default `__` suffix cannot produce. A case that fails without it:
```suggestion
]
def test_dedup_reserves_generated_names() -> None:
result = dedup(["a"] * 14 + ["a1"] * 2, suffix="")
assert len(set(result)) == len(result)
assert result[-1] == "a15"
```
--
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]
