- removed info about _local_remote_pairs from PropertyLoader.__determine_fks

- added order_by(), group_by(), having() to the list of "no offset()/limit()", [ticket:851]

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index a357aa437eb7fe8140e4da37101e881152449b3b..0a3fe8e841e5289ac56abd3700e94676fd00060f 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -602,7 +602,7 @@ class PropertyLoader(StrategizedProperty):
                 else:
                     if arg_foreign_keys:
                         raise sa_exc.ArgumentError("Could not determine relation direction for primaryjoin condition '%s', on relation %s. "
-                            "Specify _local_remote_pairs=[(local, remote), (local, remote), ...] to explicitly establish the local/remote column pairs." % (self.primaryjoin, self))
+                            "Are the columns in foreign_keys present within the given join condition ?" % (self.primaryjoin, self))
                     else:
                         raise sa_exc.ArgumentError("Could not determine relation direction for primaryjoin condition '%s', on relation %s. "
                             "Specify the foreign_keys argument to indicate which columns on the relation are foreign." % (self.primaryjoin, self))

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 6df7ea47fde56789466ad81657acc86f1e8b8ae4..429fc40c1d9171f84c716fef3f25ede657aa28ce 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -279,7 +279,7 @@ class Query(object):
         if self._limit is not None or self._offset is not None:
             # TODO: do we want from_self() to be implicit here ?  i vote explicit for the time being
             raise sa_exc.InvalidRequestError("Query.%s() being called on a Query which already has LIMIT or OFFSET applied. "
-            "To filter/join to the row-limited results of the query, call from_self() first."
+            "To modify the row-limited results of a Query, call from_self() first.  Otherwise, call %s() before limit() or offset() are applied." % (meth, meth)
             )
 
     def __no_criterion(self):
@@ -680,7 +680,7 @@ class Query(object):
         else:
             self._order_by = self._order_by + criterion
     order_by = util.array_as_starargs_decorator(order_by)
-    order_by = _generative(__no_statement_condition)(order_by)
+    order_by = _generative(__no_statement_condition, __no_limit_offset)(order_by)
 
     def group_by(self, *criterion):
         """apply one or more GROUP BY criterion to the query and return the newly resulting ``Query``"""
@@ -692,7 +692,7 @@ class Query(object):
         else:
             self._group_by = self._group_by + criterion
     group_by = util.array_as_starargs_decorator(group_by)
-    group_by = _generative(__no_statement_condition)(group_by)
+    group_by = _generative(__no_statement_condition, __no_limit_offset)(group_by)
 
     def having(self, criterion):
         """apply a HAVING criterion to the query and return the newly resulting ``Query``."""
@@ -709,7 +709,7 @@ class Query(object):
             self._having = self._having & criterion
         else:
             self._having = criterion
-    having = _generative(__no_statement_condition)(having)
+    having = _generative(__no_statement_condition, __no_limit_offset)(having)
 
     def join(self, *props, **kwargs):
         """Create a join against this ``Query`` object's criterion

diff --git a/test/orm/eager_relations.py b/test/orm/eager_relations.py
index 9934bb85ef4c651db07ee8eacff4c3c49b510aa8..f1f6ed9549f23c46b304b13a2bb4fb37c01a3f6d 100644 (file)
--- a/test/orm/eager_relations.py
+++ b/test/orm/eager_relations.py
@@ -444,7 +444,7 @@ class EagerTest(_fixtures.FixtureTest):
             l = q.limit(2).all()
             assert self.static.user_all_result[:2] == l
         else:
-            l = q.limit(2).offset(1).order_by(User.id).all()
+            l = q.order_by(User.id).limit(2).offset(1).all()
             print self.static.user_all_result[1:3]
             print l
             assert self.static.user_all_result[1:3] == l

diff --git a/test/orm/query.py b/test/orm/query.py
index d813ee864d0af1c3950d29c150c33d6224b78b8a..e23b0aadd572696959660cd76758c1cdc11e4167 100644 (file)
--- a/test/orm/query.py
+++ b/test/orm/query.py
@@ -178,12 +178,22 @@ class InvalidGenerationsTest(QueryTest):
     def test_no_limit_offset(self):
         s = create_session()
         
-        q = s.query(User).limit(2)
-        self.assertRaises(sa_exc.InvalidRequestError, q.join, "addresses")
+        for q in (
+            s.query(User).limit(2),
+            s.query(User).offset(2),
+            s.query(User).limit(2).offset(2)
+        ):
+            self.assertRaises(sa_exc.InvalidRequestError, q.join, "addresses")
+
+            self.assertRaises(sa_exc.InvalidRequestError, q.filter, User.name=='ed')
+
+            self.assertRaises(sa_exc.InvalidRequestError, q.filter_by, name='ed')
+
+            self.assertRaises(sa_exc.InvalidRequestError, q.order_by, 'foo')
 
-        self.assertRaises(sa_exc.InvalidRequestError, q.filter, User.name=='ed')
+            self.assertRaises(sa_exc.InvalidRequestError, q.group_by, 'foo')
 
-        self.assertRaises(sa_exc.InvalidRequestError, q.filter_by, name='ed')
+            self.assertRaises(sa_exc.InvalidRequestError, q.having, 'foo')
     
     def test_no_from(self):
         s = create_session()
@@ -1280,8 +1290,8 @@ class MixedEntitiesTest(QueryTest):
         oalias = aliased(Order)
         
         for q in [
-            sess.query(Order, oalias).filter(Order.user_id==oalias.user_id).filter(Order.user_id==7).filter(Order.id>oalias.id),
-            sess.query(Order, oalias)._from_self().filter(Order.user_id==oalias.user_id).filter(Order.user_id==7).filter(Order.id>oalias.id),
+            sess.query(Order, oalias).filter(Order.user_id==oalias.user_id).filter(Order.user_id==7).filter(Order.id>oalias.id).order_by(Order.id, oalias.id),
+            sess.query(Order, oalias)._from_self().filter(Order.user_id==oalias.user_id).filter(Order.user_id==7).filter(Order.id>oalias.id).order_by(Order.id, oalias.id),
             # here we go....two layers of aliasing
             sess.query(Order, oalias).filter(Order.user_id==oalias.user_id).filter(Order.user_id==7).filter(Order.id>oalias.id)._from_self().order_by(Order.id, oalias.id).limit(10).options(eagerload(Order.items)),
 
@@ -1291,7 +1301,7 @@ class MixedEntitiesTest(QueryTest):
         ]:
         
             self.assertEquals(
-            q.order_by(Order.id, oalias.id).all(),
+            q.all(),
             [
                 (Order(address_id=1,description=u'order 3',isopen=1,user_id=7,id=3), Order(address_id=1,description=u'order 1',isopen=0,user_id=7,id=1)), 
                 (Order(address_id=None,description=u'order 5',isopen=0,user_id=7,id=5), Order(address_id=1,description=u'order 1',isopen=0,user_id=7,id=1)),