Merge "restore adapter logic in ORM loading"

4 files changed, 55 insertions, 11 deletions

diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index aa7e4e5e9..7b3fa091f 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -1418,6 +1418,10 @@ class DefaultExecutionContext(interfaces.ExecutionContext):
                 strategy = _cursor._NO_CURSOR_DQL
 
             if self._is_future_result:
+                if self.root_connection.should_close_with_result:
+                    raise exc.InvalidRequestError(
+                        "can't use future_result=True with close_with_result"
+                    )
                 result = _cursor.CursorResult(
                     self, strategy, cursor_description
                 )
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index d60758ffc..e4448f953 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -127,8 +127,8 @@ class QueryContext(object):
             )
 
 
-_result_disable_adapt_to_context = util.immutabledict(
-    {"_result_disable_adapt_to_context": True}
+_orm_load_exec_options = util.immutabledict(
+    {"_result_disable_adapt_to_context": True, "future_result": True}
 )
 
 
@@ -239,16 +239,20 @@ class ORMCompileState(CompileState):
             statement._execution_options,
         )
 
-        # add _result_disable_adapt_to_context=True to execution options.
-        # this will disable the ResultSetMetadata._adapt_to_context()
-        # step which we don't need, as we have result processors cached
-        # against the original SELECT statement before caching.
+        # default execution options for ORM results:
+        # 1. _result_disable_adapt_to_context=True
+        #    this will disable the ResultSetMetadata._adapt_to_context()
+        #    step which we don't need, as we have result processors cached
+        #    against the original SELECT statement before caching.
+        # 2. future_result=True.  The ORM should **never** resolve columns
+        #    in a result set based on names, only on Column objects that
+        #    are correctly adapted to the context.   W the legacy result
+        #    it will still attempt name-based resolution and also emit a
+        #    warning.
         if not execution_options:
-            execution_options = _result_disable_adapt_to_context
+            execution_options = _orm_load_exec_options
         else:
-            execution_options = execution_options.union(
-                _result_disable_adapt_to_context
-            )
+            execution_options = execution_options.union(_orm_load_exec_options)
 
         if "yield_per" in execution_options or load_options._yield_per:
             execution_options = execution_options.union(
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index cd2ec8301..b063635ea 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -741,6 +741,30 @@ def _instance_processor(
                     )
                 else:
                     getter = None
+                    if adapter:
+                        # this logic had been removed for all 1.4 releases
+                        # up until 1.4.18; the adapter here is particularly
+                        # the compound eager adapter which isn't accommodated
+                        # in the quick_populators right now.  The "fallback"
+                        # logic below instead took over in many more cases
+                        # until issue #6596 was identified.
+
+                        # note there is still an issue where this codepath
+                        # produces no "getter" for cases where a joined-inh
+                        # mapping includes a labeled column property, meaning
+                        # KeyError is caught internally and we fall back to
+                        # _getter(col), which works anyway.   The adapter
+                        # here for joined inh without any aliasing might not
+                        # be useful.  Tests which see this include
+                        # test.orm.inheritance.test_basic ->
+                        # EagerTargetingTest.test_adapt_stringency
+                        # OptimizedLoadTest.test_column_expression_joined
+                        # PolymorphicOnNotLocalTest.test_polymorphic_on_column_prop  # noqa E501
+                        #
+
+                        adapted_col = adapter.columns[col]
+                        if adapted_col is not None:
+                            getter = result._getter(adapted_col, False)
                     if not getter:
                         getter = result._getter(col, False)
                     if getter:
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index ec11e7adf..81da15283 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1491,6 +1491,9 @@ class Session(_SessionClassMethods):
           configured with ``autocommit=True`` and does not already have a
           transaction in progress.
 
+          .. deprecated:: 1.4  this parameter is deprecated and will be removed
+             in SQLAlchemy 2.0
+
         :param execution_options: a dictionary of execution options that will
          be passed to :meth:`_engine.Connection.execution_options`, **when the
          connection is first procured only**.   If the connection is already
@@ -1673,7 +1676,16 @@ class Session(_SessionClassMethods):
 
         bind = self.get_bind(**bind_arguments)
 
-        conn = self._connection_for_bind(bind, close_with_result=True)
+        if self.autocommit:
+            # legacy stuff, we can't use future_result w/ autocommit because
+            # we rely upon close_with_result, also legacy.  it's all
+            # interrelated
+            conn = self._connection_for_bind(bind, close_with_result=True)
+            execution_options = execution_options.union(
+                dict(future_result=False)
+            )
+        else:
+            conn = self._connection_for_bind(bind)
         result = conn._execute_20(statement, params or {}, execution_options)
 
         if compile_state_cls: