Merge "Ensure col is not None when retrieving quick populators"

4 files changed, 164 insertions, 15 deletions

diff --git a/doc/build/changelog/unreleased_11/4048.rst b/doc/build/changelog/unreleased_11/4048.rst
new file mode 100644
index 000000000..612736195
--- /dev/null
+++ b/doc/build/changelog/unreleased_11/4048.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4048
+    :versions: 1.2.0b3
+
+    Fixed bug where an :func:`.undefer_group` option would not be recognized
+    if it extended from a relationship that was loading using joined eager
+    loading.  Additionally, as the bug led to excess work being performed,
+    Python function call counts are also improved by 20% within the initial
+    calculation of result set columns, complementing the joined eager load
+    improvements of :ticket:`3915`.
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index e4aea3994..cbc995489 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -322,9 +322,24 @@ def _instance_processor(
                 # be present in some unexpected way.
                 populators["expire"].append((prop.key, False))
             else:
+                getter = None
+                # the "adapter" can be here via different paths,
+                # e.g. via adapter present at setup_query or adapter
+                # applied to the query afterwards via eager load subquery.
+                # If the column here
+                # were already a product of this adapter, sending it through
+                # the adapter again can return a totally new expression that
+                # won't be recognized in the result, and the ColumnAdapter
+                # currently does not accommodate for this.   OTOH, if the
+                # column were never applied through this adapter, we may get
+                # None back, in which case we still won't get our "getter".
+                # so try both against result._getter().  See issue #4048
                 if adapter:
-                    col = adapter.columns[col]
-                getter = result._getter(col, False)
+                    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:
                     populators["quick"].append((prop.key, getter))
                 else:
diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py
index 21f40a39a..97782bffd 100644
--- a/test/orm/test_deferred.py
+++ b/test/orm/test_deferred.py
@@ -3,7 +3,7 @@ from sqlalchemy import testing, util
 from sqlalchemy.orm import mapper, deferred, defer, undefer, Load, \
     load_only, undefer_group, create_session, synonym, relationship, Session,\
     joinedload, defaultload, aliased, contains_eager, with_polymorphic, \
-    query_expression, with_expression
+    query_expression, with_expression, subqueryload
 from sqlalchemy.testing import eq_, AssertsCompiledSQL, assert_raises_message
 from test.orm import _fixtures
 from sqlalchemy.testing.schema import Column
@@ -425,6 +425,129 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
              "FROM orders WHERE :param_1 = orders.user_id ORDER BY orders.id",
              {'param_1': 7})])
 
+    def test_undefer_group_from_relationship_subqueryload(self):
+        users, Order, User, orders = \
+            (self.tables.users,
+             self.classes.Order,
+             self.classes.User,
+             self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders=relationship(Order, order_by=orders.c.id)))
+        mapper(
+            Order, orders, properties=util.OrderedDict([
+                ('userident', deferred(orders.c.user_id, group='primary')),
+                ('description', deferred(orders.c.description,
+                 group='primary')),
+                ('opened', deferred(orders.c.isopen, group='primary'))
+            ])
+        )
+
+        sess = create_session()
+        q = sess.query(User).filter(User.id == 7).options(
+            subqueryload(User.orders).undefer_group('primary')
+        )
+
+        def go():
+            result = q.all()
+            o2 = result[0].orders[1]
+            eq_(o2.opened, 1)
+            eq_(o2.userident, 7)
+            eq_(o2.description, 'order 3')
+        self.sql_eq_(go, [
+            ("SELECT users.id AS users_id, users.name AS users_name "
+             "FROM users WHERE users.id = :id_1", {"id_1": 7}),
+            ("SELECT orders.user_id AS orders_user_id, orders.description "
+             "AS orders_description, orders.isopen AS orders_isopen, "
+             "orders.id AS orders_id, orders.address_id AS orders_address_id, "
+             "anon_1.users_id AS anon_1_users_id FROM (SELECT users.id AS "
+             "users_id FROM users WHERE users.id = :id_1) AS anon_1 "
+             "JOIN orders ON anon_1.users_id = orders.user_id ORDER BY "
+             "anon_1.users_id, orders.id", [{'id_1': 7}])]
+        )
+
+    def test_undefer_group_from_relationship_joinedload(self):
+        users, Order, User, orders = \
+            (self.tables.users,
+             self.classes.Order,
+             self.classes.User,
+             self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders=relationship(Order, order_by=orders.c.id)))
+        mapper(
+            Order, orders, properties=util.OrderedDict([
+                ('userident', deferred(orders.c.user_id, group='primary')),
+                ('description', deferred(orders.c.description,
+                 group='primary')),
+                ('opened', deferred(orders.c.isopen, group='primary'))
+            ])
+        )
+
+        sess = create_session()
+        q = sess.query(User).filter(User.id == 7).options(
+            joinedload(User.orders).undefer_group('primary')
+        )
+
+        def go():
+            result = q.all()
+            o2 = result[0].orders[1]
+            eq_(o2.opened, 1)
+            eq_(o2.userident, 7)
+            eq_(o2.description, 'order 3')
+        self.sql_eq_(go, [
+            ("SELECT users.id AS users_id, users.name AS users_name, "
+             "orders_1.user_id AS orders_1_user_id, orders_1.description AS "
+             "orders_1_description, orders_1.isopen AS orders_1_isopen, "
+             "orders_1.id AS orders_1_id, orders_1.address_id AS "
+             "orders_1_address_id FROM users "
+             "LEFT OUTER JOIN orders AS orders_1 ON users.id = "
+             "orders_1.user_id WHERE users.id = :id_1 "
+             "ORDER BY orders_1.id", {"id_1": 7})]
+        )
+
+    def test_undefer_group_from_relationship_joinedload_colexpr(self):
+        users, Order, User, orders = \
+            (self.tables.users,
+             self.classes.Order,
+             self.classes.User,
+             self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders=relationship(Order, order_by=orders.c.id)))
+        mapper(
+            Order, orders, properties=util.OrderedDict([
+                ('userident', deferred(orders.c.user_id, group='primary')),
+                ('lower_desc', deferred(
+                    sa.func.lower(orders.c.description).label(None),
+                    group='primary')),
+                ('opened', deferred(orders.c.isopen, group='primary'))
+            ])
+        )
+
+        sess = create_session()
+        q = sess.query(User).filter(User.id == 7).options(
+            joinedload(User.orders).undefer_group('primary')
+        )
+
+        def go():
+            result = q.all()
+            o2 = result[0].orders[1]
+            eq_(o2.opened, 1)
+            eq_(o2.userident, 7)
+            eq_(o2.lower_desc, 'order 3')
+        self.sql_eq_(go, [
+            ("SELECT users.id AS users_id, users.name AS users_name, "
+             "orders_1.user_id AS orders_1_user_id, "
+             "lower(orders_1.description) AS lower_1, "
+             "orders_1.isopen AS orders_1_isopen, orders_1.id AS orders_1_id, "
+             "orders_1.address_id AS orders_1_address_id, "
+             "orders_1.description AS orders_1_description FROM users "
+             "LEFT OUTER JOIN orders AS orders_1 ON users.id = "
+             "orders_1.user_id WHERE users.id = :id_1 "
+             "ORDER BY orders_1.id", {"id_1": 7})]
+        )
+
     def test_undefer_star(self):
         orders, Order = self.tables.orders, self.classes.Order
 
diff --git a/test/profiles.txt b/test/profiles.txt
index e8fa350e2..e76f14cdb 100644
--- a/test/profiles.txt
+++ b/test/profiles.txt
@@ -183,18 +183,18 @@ test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_build_query 3.5_sqlite_pysq
 
 # TEST: test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results
 
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_mysql_mysqldb_dbapiunicode_cextensions 567786
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_mysql_mysqldb_dbapiunicode_nocextensions 580586
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_postgresql_psycopg2_dbapiunicode_cextensions 578444
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_postgresql_psycopg2_dbapiunicode_nocextensions 591244
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_sqlite_pysqlite_dbapiunicode_cextensions 558058
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_sqlite_pysqlite_dbapiunicode_nocextensions 570760
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_mysql_mysqldb_dbapiunicode_cextensions 570897
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_mysql_mysqldb_dbapiunicode_nocextensions 583897
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_postgresql_psycopg2_dbapiunicode_cextensions 598155
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_postgresql_psycopg2_dbapiunicode_nocextensions 611155
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_sqlite_pysqlite_dbapiunicode_cextensions 562571
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_sqlite_pysqlite_dbapiunicode_nocextensions 575669
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_mysql_mysqldb_dbapiunicode_cextensions 432486
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_mysql_mysqldb_dbapiunicode_nocextensions 445188
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_postgresql_psycopg2_dbapiunicode_cextensions 443146
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_postgresql_psycopg2_dbapiunicode_nocextensions 455848
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_sqlite_pysqlite_dbapiunicode_cextensions 422760
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 2.7_sqlite_pysqlite_dbapiunicode_nocextensions 435462
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_mysql_mysqldb_dbapiunicode_cextensions 435597
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_mysql_mysqldb_dbapiunicode_nocextensions 448499
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_postgresql_psycopg2_dbapiunicode_cextensions 462857
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_postgresql_psycopg2_dbapiunicode_nocextensions 475759
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_sqlite_pysqlite_dbapiunicode_cextensions 427371
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results 3.5_sqlite_pysqlite_dbapiunicode_nocextensions 440371
 
 # TEST: test.aaa_profiling.test_orm.LoadManyToOneFromIdentityTest.test_many_to_one_load_identity