Merge lp:~therve/storm/resultset-select-copy into lp:storm

[Thomas Herve]

The branches fixes copy by giving a real copy of the _select attribute, instead of a reference to the other ReferenceSet _select attribute.

[Jamu Kakar (jkakar)]

Looks good to me, +1!

review: Approve

[Gustavo Niemeyer (niemeyer)]

Looks good to me as well. Just one detail if you don't mind:

[1]

Can you please add some docs right above the copying, pointing out the entanglement between the few pieces which create the bug, and which require the copying to happen? This will be very valuable in a week, when we don't recall what the heck this is about anymore. :-)

review: Approve

[Gustavo Niemeyer (niemeyer)]

Actually, the comment may be as simple as this:

"""
This expression must be copied because we may have to change it in-place inside _get_select().
"""

[lp:~therve/storm/resultset-select-copy]

375. By Thomas Herve on 2010-09-01

Add a nice comment explaining why we copy the _select attribute.

=== modified file 'storm/store.py'
--- storm/store.py	2010-08-19 07:43:34 +0000
+++ storm/store.py	2010-09-01 15:02:59 +0000
@@ -927,6 +927,10 @@
         """
         result_set = object.__new__(self.__class__)
         result_set.__dict__.update(self.__dict__)
+        if self._select is not Undef:
+            # This expression must be copied because we may have to change it
+            # in-place inside _get_select().
+            result_set._select = copy(self._select)
         return result_set
 
     def config(self, distinct=None, offset=None, limit=None):

=== modified file 'tests/store/base.py'
--- tests/store/base.py	2010-08-19 07:45:09 +0000
+++ tests/store/base.py	2010-09-01 15:02:59 +0000
@@ -940,6 +940,21 @@
     def test_find_count(self):
         self.assertEquals(self.store.find(Foo).count(), 3)
 
+    def test_find_count_after_slice(self):
+        """
+        When we slice a ResultSet obtained after a set operation (like union),
+        we get a fresh select that doesn't modify the limit and offset
+        attribute of the original ResultSet.
+        """
+        result1 = self.store.find(Foo, Foo.id == 10)
+        result2 = self.store.find(Foo, Foo.id == 20)
+        result3 = result1.union(result2)
+        result3.order_by(Foo.id)
+        self.assertEquals(result3.count(), 2)
+
+        result_slice = list(result3[:2])
+        self.assertEquals(result3.count(), 2)
+
     def test_find_count_column(self):
         self.assertEquals(self.store.find(Link).count(Link.foo_id), 6)