Skip to content

Commit 77a6874

Browse files
committed
FIX: Make RowMapping lookup/membership agree with iteration
RowMapping.__getitem__ delegated to Row.__getitem__, which resolves case-insensitive names (lowercase mode) and catalog aliases that __iter__ never yields, violating the Mapping contract (e.g. 'MixedCase' in row._mapping was True while list(row._mapping) had only 'mixedcase'). Restrict the view's lookup/membership to the canonical _mapping_keys() so keys, 'in' and '[]' always agree; Row-level case-insensitive access is unchanged. Also document duplicate-label, reserved-name, and fallback-ordering caveats, and add lowercase + catalog-alias regressions.
1 parent d1d2684 commit 77a6874

3 files changed

Lines changed: 124 additions & 18 deletions

File tree

‎mssql_python/row.py‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -236,21 +236,43 @@ def __getattr__(self, name: str) -> Any:
236236

237237
@property
238238
def _mapping(self) -> "RowMapping":
239-
"""Read-only dict-like view (column name -> value) over this row.
240-
241-
Returns a ``collections.abc.Mapping``; use ``dict(row._mapping)`` for a plain
242-
dict, ``row._mapping.items()`` for name/value pairs, and ``iter(row._mapping)``
243-
for column names. Names are order-preserving and de-duplicated (last column
244-
wins for a repeated name, matching subscript and attribute access).
239+
"""Read-only ``dict``-like view (column name -> value) over this row.
240+
241+
Returns a :class:`RowMapping` (a ``collections.abc.Mapping``). Typical use::
242+
243+
row = cursor.fetchone()
244+
dict(row._mapping) # {'id': 1, 'name': 'Alice'}
245+
for name, value in row._mapping.items():
246+
...
247+
row._mapping["name"] # value by column name
248+
"name" in row._mapping # membership by column name
249+
250+
Semantics and caveats:
251+
252+
- Keys are the result set's column names, order-preserving and
253+
de-duplicated: when a name repeats, one key is kept and the last column
254+
with that name supplies its value (matching ``row[name]`` / ``row.name``).
255+
Every duplicate value stays reachable positionally via ``row[i]``.
256+
- Lookup and membership use the canonical column names exactly, so the key
257+
set, ``in`` and ``[]`` always agree. Unlike ``row[name]`` / ``row.name``,
258+
the view does NOT resolve case-insensitive names or catalog aliases; with
259+
``lowercase=True`` the keys are the lowercased names.
260+
- ``_mapping`` is a property, so a column literally named ``_mapping`` is
261+
shadowed: read it with ``row["_mapping"]`` or ``row._mapping["_mapping"]``.
245262
"""
246263
return RowMapping(self)
247264

248265
def _mapping_keys(self) -> tuple:
249266
"""Canonical, order-preserving column names backing ``_mapping``.
250267
251-
Prefers the names snapshotted once by the cursor for the result set. When a
252-
row was built without that snapshot, reconstructs names from ``_column_map``
253-
(one name per column index); returns ``()`` when neither is available.
268+
Prefers the names snapshotted once by the cursor for the result set, which
269+
preserve the result set's column order. When a row was built without that
270+
snapshot (e.g. a direct ``Row(values, column_map)`` construction), names are
271+
reconstructed from ``_column_map`` in column-index order; that order can
272+
differ from ``_column_map``'s insertion order, so a directly-constructed row
273+
may key differently from an otherwise-equivalent cursor row. Returns ``()``
274+
when neither source is available. Normal cursor fetches always supply the
275+
snapshot.
254276
"""
255277
if self._column_names is not None:
256278
return self._column_names
@@ -310,10 +332,11 @@ def __repr__(self) -> str:
310332
class RowMapping(Mapping):
311333
"""Read-only ``Mapping`` view over a :class:`Row` (column name -> value).
312334
313-
Created via :attr:`Row._mapping`. Keys are the row's column names, order-
314-
preserving and de-duplicated (last column wins for a repeated name, matching
315-
``row[name]`` / ``row.name``). The view reflects the row it wraps and copies
316-
no values.
335+
Created via :attr:`Row._mapping`. Keys are the row's canonical column names,
336+
order-preserving and de-duplicated (last column wins for a repeated name).
337+
Lookup and membership use those names exactly -- no case-insensitive or catalog
338+
alias resolution -- so iteration, ``in`` and ``[]`` always agree. The view
339+
reflects the row it wraps and copies no values.
317340
"""
318341

319342
__slots__ = ("_row",)
@@ -322,11 +345,13 @@ def __init__(self, row: "Row") -> None:
322345
self._row = row
323346

324347
def __getitem__(self, key: str) -> Any:
325-
if isinstance(key, str):
326-
try:
327-
return self._row[key]
328-
except KeyError:
329-
raise KeyError(key) from None
348+
# Restrict lookups to the canonical column names yielded by __iter__ so
349+
# membership and lookup agree with iteration (proper Mapping semantics).
350+
# Row.__getitem__ additionally accepts case-insensitive names and catalog
351+
# aliases, but those are not iterated keys, so the view must not resolve
352+
# them here.
353+
if isinstance(key, str) and key in self._row._mapping_keys():
354+
return self._row[key]
330355
raise KeyError(key)
331356

332357
def __iter__(self):

‎tests/test_001_globals.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,3 +1059,41 @@ def test_row_string_key_case_insensitive_with_lowercase():
10591059
# Non-existent attribute raises AttributeError
10601060
with pytest.raises(AttributeError):
10611061
row.nonexistent
1062+
1063+
1064+
def test_row_mapping_lookup_agrees_with_iteration_aliases():
1065+
"""RowMapping ignores case-insensitive names and catalog aliases (Mapping contract).
1066+
1067+
Metadata result sets store several keys per column index in _column_map (exact,
1068+
lowercase, and ODBC 2.x/3.x aliases). The mapping view must expose only the
1069+
canonical snapshot names it iterates, so lookup and membership never accept a
1070+
name the view does not yield.
1071+
"""
1072+
from mssql_python.row import Row
1073+
1074+
# One physical column at index 0, reachable in _column_map under several names.
1075+
column_map = {"TABLE_NAME": 0, "table_name": 0, "TABLE_QUALIFIER": 0}
1076+
row = Row(
1077+
["dbo"],
1078+
column_map,
1079+
cursor=None,
1080+
column_names=("TABLE_NAME",),
1081+
)
1082+
mapping = row._mapping
1083+
1084+
# Canonical key set == iteration == the snapshot.
1085+
assert list(mapping) == ["TABLE_NAME"]
1086+
assert dict(mapping) == {"TABLE_NAME": "dbo"}
1087+
assert mapping["TABLE_NAME"] == "dbo"
1088+
assert "TABLE_NAME" in mapping
1089+
1090+
# Row still resolves the aliases directly (unchanged behavior)...
1091+
assert row["table_name"] == "dbo"
1092+
assert row["TABLE_QUALIFIER"] == "dbo"
1093+
# ...but the mapping view does NOT, so its keys agree with membership/lookup.
1094+
assert "table_name" not in mapping
1095+
assert "TABLE_QUALIFIER" not in mapping
1096+
with pytest.raises(KeyError):
1097+
mapping["table_name"]
1098+
assert mapping.get("TABLE_QUALIFIER") is None
1099+
assert mapping.get("table_name", "fallback") == "fallback"

‎tests/test_004_cursor.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3929,6 +3929,49 @@ def test_row_mapping_lowercase_setting(db_connection):
39293929
cursor.close()
39303930

39313931

3932+
def test_row_mapping_lookup_agrees_with_iteration_lowercase(db_connection):
3933+
"""_mapping lookup/membership must agree with iteration (Mapping contract).
3934+
3935+
With lowercase=True the canonical keys are lowercased. Row itself still
3936+
resolves the original mixed-case name (case-insensitive), but the mapping
3937+
view must NOT: its keys, ``in`` and ``[]`` are restricted to the iterated
3938+
canonical names, so it can never accept a name it does not yield.
3939+
"""
3940+
original = mssql_python.lowercase
3941+
cursor = None
3942+
try:
3943+
mssql_python.lowercase = True
3944+
cursor = db_connection.cursor()
3945+
cursor.execute("SELECT 1 AS MixedCase")
3946+
row = cursor.fetchone()
3947+
mapping = row._mapping
3948+
3949+
# Row-level access is unchanged: case-insensitive name still resolves.
3950+
assert row["MixedCase"] == 1
3951+
assert row.MixedCase == 1
3952+
3953+
# The mapping view yields only the canonical lowercased key.
3954+
assert list(mapping) == ["mixedcase"]
3955+
assert mapping["mixedcase"] == 1
3956+
3957+
# The non-canonical mixed-case name is NOT a member and NOT subscriptable,
3958+
# so the key set agrees with membership and lookup.
3959+
assert "MixedCase" not in mapping
3960+
with pytest.raises(KeyError):
3961+
mapping["MixedCase"]
3962+
assert mapping.get("MixedCase") is None
3963+
assert mapping.get("MixedCase", "fallback") == "fallback"
3964+
3965+
# Every iterated key is a member and is subscriptable (contract holds).
3966+
for name in mapping:
3967+
assert name in mapping
3968+
assert mapping[name] == mapping.get(name)
3969+
finally:
3970+
mssql_python.lowercase = original
3971+
if cursor is not None:
3972+
cursor.close()
3973+
3974+
39323975
def test_row_comparison_with_list(cursor, db_connection):
39333976
"""Test comparing Row objects with lists (__eq__ method)"""
39343977
try:

0 commit comments

Comments
 (0)