From: me-saurabhkohli Date: Fri, 29 May 2026 20:03:40 +0000 (-0400) Subject: Add ambiguous column support to SimpleResultMetaData X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=9fcdaa5bbe04354f3cc5bd0bbe20e28239ae27f8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add ambiguous column support to SimpleResultMetaData Fixed issue where :meth:`.Result.freeze` would lose track of ambiguous column names present in the original :class:`.CursorResult`, causing key-based access on the thawed result to silently return a value instead of raising :class:`.InvalidRequestError`. The :class:`.SimpleResultMetaData` now accepts and propagates ambiguous key information so that frozen, thawed, and pickled results raise consistently for duplicate column names. Pull request courtesy Saurabh Kohli. Fixes: #9427 Closes: #13335 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13335 Pull-request-sha: c03904ece298493ca69bf6e9cbdae23c7fb6a7b0 Change-Id: Ia184f77b442b069e6f9a4f94a967ead41a1704b6 (cherry picked from commit 4fb459aaf05dd9c31ce3ece57c1bbf81ca9855de) --- diff --git a/doc/build/changelog/unreleased_20/9427.rst b/doc/build/changelog/unreleased_20/9427.rst new file mode 100644 index 0000000000..a601fe9977 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9427.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, engine + :tickets: 9427 + + Fixed issue where :meth:`.Result.freeze` would lose track of ambiguous + column names present in the original :class:`.CursorResult`, causing + key-based access on the thawed result to silently return a value instead of + raising :class:`.InvalidRequestError`. The + :class:`.SimpleResultMetaData` now accepts and propagates ambiguous key + information so that frozen, thawed, and pickled results raise consistently + for duplicate column names. Pull request courtesy Saurabh Kohli. diff --git a/lib/sqlalchemy/engine/cursor.py b/lib/sqlalchemy/engine/cursor.py index 15ae1d0c74..11f4c521ef 100644 --- a/lib/sqlalchemy/engine/cursor.py +++ b/lib/sqlalchemy/engine/cursor.py @@ -183,9 +183,15 @@ class CursorResultMetaData(ResultMetaData): return key in self._keymap def _for_freeze(self) -> ResultMetaData: + ambiguous = { + rec[MD_LOOKUP_KEY] + for rec in self._keymap.values() + if rec[MD_INDEX] is None + } return SimpleResultMetaData( self._keys, extra=[self._keymap[key][MD_OBJECTS] for key in self._keys], + _ambiguous_keys=frozenset(ambiguous) if ambiguous else None, ) def _make_new_metadata( diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py index f4f968ed53..341554e591 100644 --- a/lib/sqlalchemy/engine/result.py +++ b/lib/sqlalchemy/engine/result.py @@ -267,6 +267,7 @@ class SimpleResultMetaData(ResultMetaData): "_translated_indexes", "_unique_filters", "_key_to_index", + "_ambiguous_keys", ) _keys: Sequence[str] @@ -279,6 +280,7 @@ class SimpleResultMetaData(ResultMetaData): _tuplefilter: Optional[_TupleGetterType] = None, _translated_indexes: Optional[Sequence[int]] = None, _unique_filters: Optional[Sequence[Callable[[Any], Any]]] = None, + _ambiguous_keys: Optional[frozenset[str]] = None, ): self._keys = list(keys) self._tuplefilter = _tuplefilter @@ -300,8 +302,15 @@ class SimpleResultMetaData(ResultMetaData): self._keymap = {key: rec for keys, rec in recs_names for key in keys} + if _ambiguous_keys: + for name in _ambiguous_keys.intersection(self._keymap): + rec = self._keymap[name] + self._keymap[name] = (None,) + rec[1:] + self._processors = _processors + self._ambiguous_keys = _ambiguous_keys + self._key_to_index = self._make_key_to_index(self._keymap, 0) def _has_key(self, key: object) -> bool: @@ -318,12 +327,14 @@ class SimpleResultMetaData(ResultMetaData): self._keys, extra=[self._keymap[key][2] for key in self._keys], _unique_filters=unique_filters, + _ambiguous_keys=self._ambiguous_keys, ) def __getstate__(self) -> Dict[str, Any]: return { "_keys": self._keys, "_translated_indexes": self._translated_indexes, + "_ambiguous_keys": self._ambiguous_keys, } def __setstate__(self, state: Dict[str, Any]) -> None: @@ -336,6 +347,7 @@ class SimpleResultMetaData(ResultMetaData): state["_keys"], _translated_indexes=_translated_indexes, _tuplefilter=_tuplefilter, + _ambiguous_keys=state.get("_ambiguous_keys"), ) def _index_for_key(self, key: Any, raiseerr: bool = True) -> int: @@ -346,9 +358,21 @@ class SimpleResultMetaData(ResultMetaData): except KeyError as ke: rec = self._key_fallback(key, ke, raiseerr) + if rec[0] is None: + self._raise_for_ambiguous_column_name(rec) return rec[0] # type: ignore[no-any-return] + def _raise_for_ambiguous_column_name( + self, rec: _KeyMapRecType + ) -> NoReturn: + raise exc.InvalidRequestError( + "Ambiguous column name '%s' in " + "result set column descriptions" % rec[1] + ) + def _indexes_for_keys(self, keys: Sequence[Any]) -> Sequence[int]: + # only used by the ORM with Column objects; does not need + # ambiguous column name support return [self._keymap[key][0] for key in keys] def _metadata_for_keys( @@ -363,6 +387,9 @@ class SimpleResultMetaData(ResultMetaData): except KeyError as ke: rec = self._key_fallback(key, ke, True) + if rec[0] is None: + self._raise_for_ambiguous_column_name(rec) + yield rec def _reduce(self, keys: Sequence[Any]) -> ResultMetaData: diff --git a/test/base/test_result.py b/test/base/test_result.py index 78117e3228..929cfd5a14 100644 --- a/test/base/test_result.py +++ b/test/base/test_result.py @@ -959,6 +959,22 @@ class ResultTest(fixtures.TestBase): r2 = frozen().scalars(1).unique() eq_(r2.fetchall(), [1, 3]) + def test_ambiguous_key_raises(self): + meta = result.SimpleResultMetaData( + ["x", "x"], _ambiguous_keys=frozenset(["x"]) + ) + res_obj = result.IteratorResult(meta, iter([(4, 2)])) + row = res_obj.fetchone() + + eq_(row[0], 4) + eq_(row[1], 2) + + assert_raises_message( + exc.InvalidRequestError, + "Ambiguous column name 'x'", + lambda: row._mapping["x"], + ) + class MergeResultTest(fixtures.TestBase): @testing.fixture diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py index 4d041ffbb5..74725a0368 100644 --- a/test/sql/test_resultset.py +++ b/test/sql/test_resultset.py @@ -1191,7 +1191,10 @@ class CursorResultTest(fixtures.TablesTest): ) @testing.requires.duplicate_names_in_cursor_description - def test_ambiguous_column(self, connection): + @testing.variation( + "scenario", ["plain", "pickled", "frozen", "frozen_pickled"] + ) + def test_ambiguous_column(self, connection, scenario): users = self.tables.users addresses = self.tables.addresses @@ -1201,14 +1204,35 @@ class CursorResultTest(fixtures.TablesTest): .select() .set_label_style(LABEL_STYLE_NONE) ) + + if scenario.frozen or scenario.frozen_pickled: + frozen = result.freeze() + result = frozen() + r = result.first() + if scenario.pickled or scenario.frozen_pickled: + r = pickle.loads(pickle.dumps(r)) + assert_raises_message( exc.InvalidRequestError, "Ambiguous column name", lambda: r._mapping["user_id"], ) + @testing.requires.duplicate_names_in_cursor_description + def test_ambiguous_column_getter(self, connection): + users = self.tables.users + addresses = self.tables.addresses + + connection.execute(users.insert(), dict(user_id=1, user_name="john")) + result = connection.execute( + users.outerjoin(addresses) + .select() + .set_label_style(LABEL_STYLE_NONE) + ) + r = result.first() + assert_raises_message( exc.InvalidRequestError, "Ambiguous column name", @@ -1222,8 +1246,19 @@ class CursorResultTest(fixtures.TablesTest): eq_(r._mapping[users.c.user_id], 1) eq_(r._mapping[addresses.c.user_id], None) - # try to trick it - fake_table isn't in the result! - # we get the correct error + @testing.requires.duplicate_names_in_cursor_description + def test_ambiguous_column_fake_table(self, connection): + users = self.tables.users + addresses = self.tables.addresses + + connection.execute(users.insert(), dict(user_id=1, user_name="john")) + result = connection.execute( + users.outerjoin(addresses) + .select() + .set_label_style(LABEL_STYLE_NONE) + ) + r = result.first() + fake_table = Table("fake", MetaData(), Column("user_id", Integer)) assert_raises_message( exc.InvalidRequestError, @@ -1231,13 +1266,6 @@ class CursorResultTest(fixtures.TablesTest): lambda: r._mapping[fake_table.c.user_id], ) - r = pickle.loads(pickle.dumps(r)) - assert_raises_message( - exc.InvalidRequestError, - "Ambiguous column name", - lambda: r._mapping["user_id"], - ) - @testing.requires.duplicate_names_in_cursor_description def test_ambiguous_column_by_col(self, connection): users = self.tables.users