From: Mike Bayer Date: Mon, 19 Sep 2022 13:40:40 +0000 (-0400) Subject: break out text() from TextualSelect for col matching X-Git-Tag: rel_2_0_0b1~52^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c9af2ebf5e5d288aea3a3a26bdf950e08ac4f927;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git break out text() from TextualSelect for col matching Fixed issue where mixing "*" with additional explicitly-named column expressions within the columns clause of a :func:`_sql.select` construct would cause result-column targeting to sometimes consider the label name or other non-repeated names to be an ambiguous target. Fixes: #8536 Change-Id: I3c845eaf571033e54c9208762344f67f4351ac3a --- diff --git a/doc/build/changelog/unreleased_14/8536.rst b/doc/build/changelog/unreleased_14/8536.rst new file mode 100644 index 0000000000..d7b5283cde --- /dev/null +++ b/doc/build/changelog/unreleased_14/8536.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, engine + :tickets: 8536 + + Fixed issue where mixing "*" with additional explicitly-named column + expressions within the columns clause of a :func:`_sql.select` construct + would cause result-column targeting to sometimes consider the label name or + other non-repeated names to be an ambiguous target. diff --git a/lib/sqlalchemy/engine/cursor.py b/lib/sqlalchemy/engine/cursor.py index 0204fbbbb8..8840b59161 100644 --- a/lib/sqlalchemy/engine/cursor.py +++ b/lib/sqlalchemy/engine/cursor.py @@ -237,6 +237,7 @@ class CursorResultMetaData(ResultMetaData): result_columns, cols_are_ordered, textual_ordered, + ad_hoc_textual, loose_column_name_matching, ) = context.result_column_struct num_ctx_cols = len(result_columns) @@ -245,6 +246,8 @@ class CursorResultMetaData(ResultMetaData): cols_are_ordered ) = ( num_ctx_cols + ) = ( + ad_hoc_textual ) = loose_column_name_matching = textual_ordered = False # merge cursor.description with the column info @@ -256,6 +259,7 @@ class CursorResultMetaData(ResultMetaData): num_ctx_cols, cols_are_ordered, textual_ordered, + ad_hoc_textual, loose_column_name_matching, ) @@ -282,8 +286,11 @@ class CursorResultMetaData(ResultMetaData): } if len(by_key) != num_ctx_cols: - # if by-primary-string dictionary smaller (or bigger?!) than - # number of columns, assume we have dupes, rewrite + # if by-primary-string dictionary smaller than + # number of columns, assume we have dupes; (this check + # is also in place if string dictionary is bigger, as + # can occur when '*' was used as one of the compiled columns, + # which may or may not be suggestive of dupes), rewrite # dupe records with "None" for index which results in # ambiguous column exception when accessed. # @@ -368,6 +375,7 @@ class CursorResultMetaData(ResultMetaData): num_ctx_cols, cols_are_ordered, textual_ordered, + ad_hoc_textual, loose_column_name_matching, ): """Merge a cursor.description with compiled result column information. @@ -461,7 +469,9 @@ class CursorResultMetaData(ResultMetaData): # name-based or text-positional cases, where we need # to read cursor.description names - if textual_ordered: + if textual_ordered or ( + ad_hoc_textual and len(cursor_description) == num_ctx_cols + ): self._safe_for_cache = True # textual positional case raw_iterator = self._merge_textual_cols_by_position( diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 9ad0ebbfc0..3a53f8157f 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -939,7 +939,7 @@ class DefaultExecutionContext(ExecutionContext): executemany = False compiled: Optional[Compiled] = None result_column_struct: Optional[ - Tuple[List[ResultColumnsEntry], bool, bool, bool] + Tuple[List[ResultColumnsEntry], bool, bool, bool, bool] ] = None returned_default_rows: Optional[Sequence[Row[Any]]] = None @@ -1057,6 +1057,7 @@ class DefaultExecutionContext(ExecutionContext): compiled._result_columns, compiled._ordered_columns, compiled._textual_ordered_columns, + compiled._ad_hoc_textual, compiled._loose_column_name_matching, ) diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 45b5eab562..1d13ffa9a4 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -838,6 +838,20 @@ class SQLCompiler(Compiled): _textual_ordered_columns: bool = False """tell the result object that the column names as rendered are important, but they are also "ordered" vs. what is in the compiled object here. + + As of 1.4.42 this condition is only present when the statement is a + TextualSelect, e.g. text("....").columns(...), where it is required + that the columns are considered positionally and not by name. + + """ + + _ad_hoc_textual: bool = False + """tell the result that we encountered text() or '*' constructs in the + middle of the result columns, but we also have compiled columns, so + if the number of columns in cursor.description does not match how many + expressions we have, that means we can't rely on positional at all and + should match on name. + """ _ordered_columns: bool = True @@ -3592,7 +3606,7 @@ class SQLCompiler(Compiled): ) -> None: if keyname is None or keyname == "*": self._ordered_columns = False - self._textual_ordered_columns = True + self._ad_hoc_textual = True if type_._is_tuple_type: raise exc.CompileError( "Most backends don't support SELECTing " diff --git a/test/requirements.py b/test/requirements.py index e7de9f7954..031e0eb4ae 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -351,6 +351,17 @@ class DefaultRequirements(SuiteRequirements): return skip_if(["+pyodbc"], "no driver support") + @property + def select_star_mixed(self): + r"""target supports expressions like "SELECT x, y, \*, z FROM table" + + apparently MySQL / MariaDB, Oracle doesn't handle this. + + We only need a few backends so just cover SQLite / PG + + """ + return only_on(["sqlite", "postgresql"]) + @property def independent_connections(self): """ diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py index 5df5c8a736..42cf31bf54 100644 --- a/test/sql/test_resultset.py +++ b/test/sql/test_resultset.py @@ -1158,6 +1158,50 @@ class CursorResultTest(fixtures.TablesTest): set([True]), ) + @testing.combinations( + (("name_label", "*"), False), + (("*", "name_label"), False), + (("user_id", "name_label", "user_name"), False), + (("user_id", "name_label", "*", "user_name"), True), + argnames="cols,other_cols_are_ambiguous", + ) + @testing.requires.select_star_mixed + def test_label_against_star( + self, connection, cols, other_cols_are_ambiguous + ): + """test #8536""" + users = self.tables.users + + connection.execute(users.insert(), dict(user_id=1, user_name="john")) + + stmt = select( + *[ + text("*") + if colname == "*" + else users.c.user_name.label("name_label") + if colname == "name_label" + else users.c[colname] + for colname in cols + ] + ) + + row = connection.execute(stmt).first() + + eq_(row._mapping["name_label"], "john") + + if other_cols_are_ambiguous: + with expect_raises_message( + exc.InvalidRequestError, "Ambiguous column name" + ): + row._mapping["user_id"] + with expect_raises_message( + exc.InvalidRequestError, "Ambiguous column name" + ): + row._mapping["user_name"] + else: + eq_(row._mapping["user_id"], 1) + eq_(row._mapping["user_name"], "john") + def test_loose_matching_one(self, connection): users = self.tables.users addresses = self.tables.addresses