Apply consistent labeling for all future style ORM queries

Fixed issue in new 1.4/2.0 style ORM queries where a statement-level label
style would not be preserved in the keys used by result rows; this has been
applied to all combinations of Core/ORM columns / session vs. connection
etc. so that the linkage from statement to result row is the same in all
cases.

also repairs a cache key bug where query.from_statement()
vs. select().from_statement() would not be disambiguated; the
compile options were not included in the cache key for
FromStatement.

Fixes: #5933
Change-Id: I22f6cf0f0b3360e55299cdcb2452cead2b2458ea

4 files changed, 78 insertions, 27 deletions

diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index f9a0b72fe..621ed826c 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -42,6 +42,9 @@ _path_registry = PathRegistry.root
 _EMPTY_DICT = util.immutabledict()
 
 
+LABEL_STYLE_LEGACY_ORM = util.symbol("LABEL_STYLE_LEGACY_ORM")
+
+
 class QueryContext(object):
     __slots__ = (
         "compile_state",
@@ -175,6 +178,21 @@ class ORMCompileState(CompileState):
         raise NotImplementedError()
 
     @classmethod
+    def _column_naming_convention(cls, label_style, legacy):
+
+        if legacy:
+
+            def name(col, col_name=None):
+                if col_name:
+                    return col_name
+                else:
+                    return getattr(col, "key")
+
+            return name
+        else:
+            return SelectState._column_naming_convention(label_style)
+
+    @classmethod
     def create_for_statement(cls, statement_container, compiler, **kw):
         """Create a context for a statement given a :class:`.Compiler`.
 
@@ -345,6 +363,25 @@ class ORMFromStatementCompileState(ORMCompileState):
 
         self.compile_options = statement_container._compile_options
 
+        if (
+            self.use_legacy_query_style
+            and isinstance(statement, expression.SelectBase)
+            and not statement._is_textual
+            and statement._label_style is LABEL_STYLE_NONE
+        ):
+            self.statement = statement.set_label_style(
+                LABEL_STYLE_TABLENAME_PLUS_COL
+            )
+        else:
+            self.statement = statement
+
+        self._label_convention = self._column_naming_convention(
+            statement._label_style
+            if not statement._is_textual
+            else LABEL_STYLE_NONE,
+            self.use_legacy_query_style,
+        )
+
         _QueryEntity.to_compile_state(self, statement_container._raw_columns)
 
         self.current_path = statement_container._compile_options._current_path
@@ -370,16 +407,6 @@ class ORMFromStatementCompileState(ORMCompileState):
         self.create_eager_joins = []
         self._fallback_from_clauses = []
 
-        if (
-            isinstance(statement, expression.SelectBase)
-            and not statement._is_textual
-            and statement._label_style is util.symbol("LABEL_STYLE_NONE")
-        ):
-            self.statement = statement.set_label_style(
-                LABEL_STYLE_TABLENAME_PLUS_COL
-            )
-        else:
-            self.statement = statement
         self.order_by = None
 
         if isinstance(self.statement, expression.TextClause):
@@ -499,20 +526,27 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
 
         self.compile_options = select_statement._compile_options
 
-        _QueryEntity.to_compile_state(self, select_statement._raw_columns)
-
         # determine label style.   we can make different decisions here.
         # at the moment, trying to see if we can always use DISAMBIGUATE_ONLY
         # rather than LABEL_STYLE_NONE, and if we can use disambiguate style
         # for new style ORM selects too.
-        if self.select_statement._label_style is LABEL_STYLE_NONE:
-            if self.use_legacy_query_style and not self.for_statement:
+        if (
+            self.use_legacy_query_style
+            and self.select_statement._label_style is LABEL_STYLE_LEGACY_ORM
+        ):
+            if not self.for_statement:
                 self.label_style = LABEL_STYLE_TABLENAME_PLUS_COL
             else:
                 self.label_style = LABEL_STYLE_DISAMBIGUATE_ONLY
         else:
             self.label_style = self.select_statement._label_style
 
+        self._label_convention = self._column_naming_convention(
+            statement._label_style, self.use_legacy_query_style
+        )
+
+        _QueryEntity.to_compile_state(self, select_statement._raw_columns)
+
         self.current_path = select_statement._compile_options._current_path
 
         self.eager_order_by = ()
@@ -685,7 +719,7 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
                 )
 
     @classmethod
-    def _create_entities_collection(cls, query):
+    def _create_entities_collection(cls, query, legacy):
         """Creates a partial ORMSelectCompileState that includes
         the full collection of _MapperEntity and other _QueryEntity objects.
 
@@ -710,6 +744,10 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
             )
             self._setup_with_polymorphics()
 
+        self._label_convention = self._column_naming_convention(
+            query._label_style, legacy
+        )
+
         # entities will also set up polymorphic adapters for mappers
         # that have with_polymorphic configured
         _QueryEntity.to_compile_state(self, query._raw_columns)
@@ -1979,10 +2017,12 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
                 self._where_criteria += (crit,)
 
 
-def _column_descriptions(query_or_select_stmt, compile_state=None):
+def _column_descriptions(
+    query_or_select_stmt, compile_state=None, legacy=False
+):
     if compile_state is None:
         compile_state = ORMSelectCompileState._create_entities_collection(
-            query_or_select_stmt
+            query_or_select_stmt, legacy=legacy
         )
     ctx = compile_state
     return [
@@ -2518,7 +2558,8 @@ class _RawColumnEntity(_ColumnEntity):
 
     def __init__(self, compile_state, column, parent_bundle=None):
         self.expr = column
-        self._label_name = getattr(column, "key", None)
+
+        self._label_name = compile_state._label_convention(column)
 
         if parent_bundle:
             parent_bundle._entities.append(self)
@@ -2582,13 +2623,17 @@ class _ORMColumnEntity(_ColumnEntity):
         # a column if it was acquired using the class' adapter directly,
         # such as using AliasedInsp._adapt_element().  this occurs
         # within internal loaders.
-        self._label_name = _label_name = annotations.get("orm_key", None)
-        if _label_name:
-            self.expr = getattr(_entity.entity, _label_name)
+
+        orm_key = annotations.get("orm_key", None)
+        if orm_key:
+            self.expr = getattr(_entity.entity, orm_key)
         else:
-            self._label_name = getattr(column, "key", None)
             self.expr = column
 
+        self._label_name = compile_state._label_convention(
+            column, col_name=orm_key
+        )
+
         _entity._post_inspect
         self.entity_zero = self.entity_zero_or_selectable = ezero = _entity
         self.mapper = mapper = _entity.mapper
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index a63a4236d..24751bf1d 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -252,7 +252,9 @@ def merge_result(query, iterator, load=True):
     else:
         frozen_result = None
 
-    ctx = querycontext.ORMSelectCompileState._create_entities_collection(query)
+    ctx = querycontext.ORMSelectCompileState._create_entities_collection(
+        query, True
+    )
 
     autoflush = session.autoflush
     try:
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index d36818254..30cb9e730 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -30,6 +30,7 @@ from .base import _assertions
 from .context import _column_descriptions
 from .context import _legacy_determine_last_joined_entity
 from .context import _legacy_filter_by_entity_zero
+from .context import LABEL_STYLE_LEGACY_ORM
 from .context import ORMCompileState
 from .context import ORMFromStatementCompileState
 from .context import QueryContext
@@ -59,7 +60,6 @@ from ..sql.selectable import ForUpdateArg
 from ..sql.selectable import HasHints
 from ..sql.selectable import HasPrefixes
 from ..sql.selectable import HasSuffixes
-from ..sql.selectable import LABEL_STYLE_NONE
 from ..sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL
 from ..sql.selectable import SelectStatementGrouping
 from ..sql.visitors import InternalTraversal
@@ -119,7 +119,7 @@ class Query(
     _from_obj = ()
     _setup_joins = ()
     _legacy_setup_joins = ()
-    _label_style = LABEL_STYLE_NONE
+    _label_style = LABEL_STYLE_LEGACY_ORM
 
     _compile_options = ORMCompileState.default_compile_options
 
@@ -2825,7 +2825,7 @@ class Query(
 
         """
 
-        return _column_descriptions(self)
+        return _column_descriptions(self, legacy=True)
 
     def instances(self, result_proxy, context=None):
         """Return an ORM result given a :class:`_engine.CursorResult` and
@@ -3199,6 +3199,10 @@ class FromStatement(SelectStatementGrouping, Executable):
         ("element", InternalTraversal.dp_clauseelement),
     ] + Executable._executable_traverse_internals
 
+    _cache_key_traversal = _traverse_internals + [
+        ("_compile_options", InternalTraversal.dp_has_cache_key)
+    ]
+
     def __init__(self, entities, element):
         self._raw_columns = [
             coercions.expect(
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index c80b8f5a2..51f75baf3 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1593,7 +1593,7 @@ class SubqueryLoader(PostLoader):
         # much of this we need.    in particular I can't get a test to
         # fail if the "set_base_alias" is missing and not sure why that is.
         orig_compile_state = compile_state_cls._create_entities_collection(
-            orig_query
+            orig_query, legacy=False
         )
 
         (