- Fixed up slices on Query (i.e. query[x:y]) to work properly

for zero length slices, slices with None on either end.
[ticket:1177]

diff --git a/CHANGES b/CHANGES
index adfdcf0560aa43ace12634d6beb941968e571b5e..3686034bcd8951a090c0a8b8a23f976c48faa8ca 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -27,6 +27,10 @@ CHANGES
       ability to apply default __init__ implementations on object
       subclasses.
 
+    - Fixed up slices on Query (i.e. query[x:y]) to work properly
+      for zero length slices, slices with None on either end.
+      [ticket:1177]
+
 - sql
     - column.in_(someselect) can now be used as a columns-clause
       expression without the subquery bleeding into the FROM clause

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index c2d47afeae693794b8ac30c05e3c116956549924..e25716316a85b960c14ca914da0d777c901539ba 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -927,16 +927,21 @@ class Query(object):
     def __getitem__(self, item):
         if isinstance(item, slice):
             start, stop, step = util.decode_slice(item)
-            # if we slice from the end we need to execute the query before
-            # slicing
-            if start < 0 or stop < 0:
+
+            if isinstance(stop, int) and isinstance(start, int) and stop - start <= 0:
+                return []
+            
+            # perhaps we should execute a count() here so that we
+            # can still use LIMIT/OFFSET ?
+            elif (isinstance(start, int) and start < 0) \
+                or (isinstance(stop, int) and stop < 0):
                 return list(self)[item]
+                
+            res = self.slice(start, stop)
+            if step is not None:
+                return list(res)[None:None:item.step]
             else:
-                res = self.slice(start, stop)
-                if step is not None:
-                    return list(res)[None:None:item.step]
-                else:
-                    return list(res)
+                return list(res)
         else:
             return list(self[item:item+1])[0]
 

diff --git a/test/orm/generative.py b/test/orm/generative.py
index 6b3bf8c847bc9e7660aa967bdb4e2634c9b2128d..f2d3579dbb9102c68e20bd92436c9c453e7ae6ea 100644 (file)
--- a/test/orm/generative.py
+++ b/test/orm/generative.py
@@ -41,13 +41,19 @@ class GenerativeQueryTest(_base.MappedTest):
         sess = create_session()
         query = sess.query(Foo)
         orig = query.all()
+        
         assert query[1] == orig[1]
         assert list(query[10:20]) == orig[10:20]
         assert list(query[10:]) == orig[10:]
         assert list(query[:10]) == orig[:10]
         assert list(query[:10]) == orig[:10]
+        assert list(query[5:5]) == orig[5:5]
         assert list(query[10:40:3]) == orig[10:40:3]
         assert list(query[-5:]) == orig[-5:]
+        assert list(query[-2:-5]) == orig[-2:-5]
+        assert list(query[-5:-2]) == orig[-5:-2]
+        assert list(query[:-2]) == orig[:-2]
+        
         assert query[10:20][5] == orig[10:20][5]
 
     @testing.uses_deprecated('Call to deprecated function apply_max')

diff --git a/test/orm/query.py b/test/orm/query.py
index ac961d9a0a6d99ddc80e201d4ea2e4b70cf25ff4..5d77f2d8155a43cdd5e285e9f82a955c300aedc1 100644 (file)
--- a/test/orm/query.py
+++ b/test/orm/query.py
@@ -497,13 +497,53 @@ class CompileTest(QueryTest):
         l = list(session.query(User).instances(s.execute(emailad = 'jack@bean.com')))
         assert [User(id=7)] == l
 
+# more slice tests are available in test/orm/generative.py
 class SliceTest(QueryTest):
     def test_first(self):
         assert  User(id=7) == create_session().query(User).first()
 
         assert create_session().query(User).filter(User.id==27).first() is None
 
-        # more slice tests are available in test/orm/generative.py
+    @testing.fails_on_everything_except('sqlite')
+    def test_limit_offset_applies(self):
+        """Test that the expected LIMIT/OFFSET is applied for slices.
+        
+        The LIMIT/OFFSET syntax differs slightly on all databases, and
+        query[x:y] executes immediately, so we are asserting against
+        SQL strings using sqlite's syntax.
+        
+        """
+        sess = create_session()
+        q = sess.query(User)
+        
+        self.assert_sql(testing.db, lambda: q[10:20], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users  LIMIT 10 OFFSET 10", {})
+        ])
+
+        self.assert_sql(testing.db, lambda: q[:20], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users  LIMIT 20 OFFSET 0", {})
+        ])
+
+        self.assert_sql(testing.db, lambda: q[5:], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users  LIMIT -1 OFFSET 5", {})
+        ])
+
+        self.assert_sql(testing.db, lambda: q[2:2], [])
+
+        self.assert_sql(testing.db, lambda: q[-2:-5], [])
+
+        self.assert_sql(testing.db, lambda: q[-5:-2], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users", {})
+        ])
+
+        self.assert_sql(testing.db, lambda: q[-5:], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users", {})
+        ])
+
+        self.assert_sql(testing.db, lambda: q[:], [
+            ("SELECT users.id AS users_id, users.name AS users_name FROM users", {})
+        ])
+
 
 class TextTest(QueryTest):
     def test_fulltext(self):