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_1_4_42~22 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbd90a987eea70c28dd1e2ee7007722b16ec9f74;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 (cherry picked from commit 78327d98be9236c61f950526470f29b184dabba6) --- 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 774916d95d..168e08d111 100644 --- a/lib/sqlalchemy/engine/cursor.py +++ b/lib/sqlalchemy/engine/cursor.py @@ -165,6 +165,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) @@ -173,6 +174,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 @@ -184,6 +187,7 @@ class CursorResultMetaData(ResultMetaData): num_ctx_cols, cols_are_ordered, textual_ordered, + ad_hoc_textual, loose_column_name_matching, ) @@ -214,11 +218,18 @@ class CursorResultMetaData(ResultMetaData): # column keys and other names if num_ctx_cols: - # if by-primary-string dictionary smaller (or bigger?!) than - # number of columns, assume we have dupes, rewrite - # dupe records with "None" for index which results in - # ambiguous column exception when accessed. if len(by_key) != num_ctx_cols: + # 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. + # + # this is considered to be the less common case as it is not + # common to have dupe column keys in a SELECT statement. + # # new in 1.4: get the complete set of all possible keys, # strings, objects, whatever, that are dupes across two # different records, first. @@ -291,6 +302,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. @@ -386,7 +398,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 6b58c44696..e050bea7a7 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -975,6 +975,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext): compiled._result_columns, compiled._ordered_columns, compiled._textual_ordered_columns, + compiled._ad_hoc_textual, compiled._loose_column_name_matching, ) self.isinsert = compiled.isinsert diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index c9b6ba670c..0e441fbec8 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -611,6 +611,20 @@ class SQLCompiler(Compiled): _textual_ordered_columns = 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 = 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 = True @@ -3024,7 +3038,7 @@ class SQLCompiler(Compiled): def _add_to_result_map(self, keyname, name, objects, type_): 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 68e5f8bfe2..ca074c79b2 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -363,6 +363,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 13190f915f..5d29b0b2b1 100644 --- a/test/sql/test_resultset.py +++ b/test/sql/test_resultset.py @@ -1020,6 +1020,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