- Modified the behavior of :func:`.orm.load_only` such that primary key

columns are always added to the list of columns to be "undeferred";
otherwise, the ORM can't load the row's identity.   Apparently,
one can defer the mapped primary keys and the ORM will fail, that
hasn't been changed.  But as load_only is essentially saying
"defer all but X", it's more critical that PK cols not be part of this
deferral. fixes #3080

4 files changed, 66 insertions, 8 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 0b4ad616a..4e1cddcd7 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,18 @@
     :version: 0.9.5
 
     .. change::
+        :tags: bug, orm
+        :tickets: 3080
+
+        Modified the behavior of :func:`.orm.load_only` such that primary key
+        columns are always added to the list of columns to be "undeferred";
+        otherwise, the ORM can't load the row's identity.   Apparently,
+        one can defer the mapped primary keys and the ORM will fail, that
+        hasn't been changed.  But as load_only is essentially saying
+        "defer all but X", it's more critical that PK cols not be part of this
+        deferral.
+
+    .. change::
         :tags: feature, examples
         :pullreq: bitbucket: 21
 
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 2674b9c6f..567c09fff 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -208,10 +208,24 @@ class DeferredColumnLoader(LoaderStrategy):
 
     def setup_query(self, context, entity, path, loadopt, adapter,
                                 only_load_props=None, **kwargs):
+
         if (
-                loadopt and self.group and
+            (
+                loadopt and
+                'undefer_pks' in loadopt.local_opts and
+                set(self.columns).intersection(self.parent.primary_key)
+            )
+            or
+            (
+                loadopt and
+                self.group and
                 loadopt.local_opts.get('undefer_group', False) == self.group
-            ) or (only_load_props and self.key in only_load_props):
+            )
+            or
+            (
+                only_load_props and self.key in only_load_props
+            )
+        ):
             self.parent_property._get_strategy_by_cls(ColumnLoader).\
                             setup_query(context, entity,
                                         path, loadopt, adapter, **kwargs)
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 317fc0813..eccb4f7f0 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -582,7 +582,8 @@ def load_only(loadopt, *attrs):
                 {"deferred": False, "instrument": True}
             )
     cloned.set_column_strategy("*",
-                    {"deferred": True, "instrument": True})
+                    {"deferred": True, "instrument": True},
+                    {"undefer_pks": True})
     return cloned
 
 @load_only._add_unbound_fn
diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py
index d96d420e7..1457852d8 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
-from sqlalchemy.testing import eq_, AssertsCompiledSQL
+from sqlalchemy.testing import eq_, AssertsCompiledSQL, assert_raises_message
 from test.orm import _fixtures
 from sqlalchemy.orm import strategies
 
@@ -37,6 +37,24 @@ class DeferredTest(AssertsCompiledSQL, _fixtures.FixtureTest):
              "FROM orders WHERE orders.id = :param_1",
              {'param_1':3})])
 
+    def test_defer_primary_key(self):
+        """what happens when we try to defer the primary key?"""
+
+        Order, orders = self.classes.Order, self.tables.orders
+
+
+        mapper(Order, orders, order_by=orders.c.id, properties={
+            'id': deferred(orders.c.id)})
+
+        # right now, it's not that graceful :)
+        q = create_session().query(Order)
+        assert_raises_message(
+            sa.exc.NoSuchColumnError,
+            "Could not locate",
+            q.first
+        )
+
+
     def test_unsaved(self):
         """Deferred loading does not kick in when just PK cols are set."""
 
@@ -431,7 +449,7 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
             "LEFT OUTER JOIN orders AS orders_1 ON users.id = orders_1.user_id"
             )
 
-    def test_load_only(self):
+    def test_load_only_no_pk(self):
         orders, Order = self.tables.orders, self.classes.Order
 
         mapper(Order, orders)
@@ -439,9 +457,20 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
         sess = create_session()
         q = sess.query(Order).options(load_only("isopen", "description"))
         self.assert_compile(q,
-            "SELECT orders.description AS orders_description, "
+            "SELECT orders.id AS orders_id, "
+            "orders.description AS orders_description, "
             "orders.isopen AS orders_isopen FROM orders")
 
+    def test_load_only_no_pk_rt(self):
+        orders, Order = self.tables.orders, self.classes.Order
+
+        mapper(Order, orders)
+
+        sess = create_session()
+        q = sess.query(Order).order_by(Order.id).\
+                options(load_only("isopen", "description"))
+        eq_(q.first(), Order(id=1))
+
     def test_load_only_w_deferred(self):
         orders, Order = self.tables.orders, self.classes.Order
 
@@ -456,6 +485,7 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
                 )
         self.assert_compile(q,
             "SELECT orders.description AS orders_description, "
+            "orders.id AS orders_id, "
             "orders.user_id AS orders_user_id, "
             "orders.isopen AS orders_isopen FROM orders")
 
@@ -522,7 +552,8 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
                 )
 
         self.assert_compile(q,
-            "SELECT users.name AS users_name, orders.id AS orders_id, "
+            "SELECT users.id AS users_id, users.name AS users_name, "
+            "orders.id AS orders_id, "
             "addresses.id AS addresses_id, addresses.email_address "
             "AS addresses_email_address FROM users, orders, addresses"
             )
@@ -554,7 +585,7 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest):
         # hmmmm joinedload seems to be forcing users.id into here...
         self.assert_compile(
             q,
-            "SELECT users.name AS users_name, users.id AS users_id, "
+            "SELECT users.id AS users_id, users.name AS users_name, "
             "addresses_1.id AS addresses_1_id, "
             "addresses_1.email_address AS addresses_1_email_address, "
             "orders_1.id AS orders_1_id FROM users "