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 (file)
index 0000000..d7b5283
--- /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 0204fbbbb859be99f910cd538ed6a9893f9adf2d..8840b5916138d513ad7543b381baf8f5bcf199bd 100644 (file)
--- 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 9ad0ebbfc05f84665ce77794e9a9d84f903326d8..3a53f8157f0a0dc3ada5eed77d46b1c0e811a320 100644 (file)
--- 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 45b5eab562a2511fec953e655f07d0dd2682785f..1d13ffa9a43a4122e91f718e5bb2bc0fed1fbba7 100644 (file)
--- 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 e7de9f7954972528262ec14404b665fa5d1a0bb2..031e0eb4ae7ff220575ed0c33c1b8e2775b37811 100644 (file)
--- 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 5df5c8a736863e02e1434fdcc30d41a52f671ab9..42cf31bf54d9c1ee85e04ffff784e21479dd7495 100644 (file)
--- 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