Slicing a ResultSet breaks subsequent count() calls.

OK, here is my real problem. 'foo' is a complex ResultSet.

>>> len(foo)
9
>>> tmp = foo[1:3]
>>> len(foo)
Traceback (most recent call last):
...
FeatureError: __contains__() does not yet support set expressions that combine ORDER BY with LIMIT/OFFSET

Here's the problem. When I slice the ResultSet, ResultSet.__getitem__ makes a copy of the ResultSet and configures the copy with the desired offset and limit. But the original and the copy share the same .select member. (This is an object from the storm.expr module.) When I call len(foo), ._select.limit and ._select.offset are populated for both the original and the copy.

When I call len() the second time. some code in _get_select() runs that looks like it should normalize the data.

        if self._select is not Undef:
            if self._order_by is not Undef:
                self._select.order_by = self._order_by
            if self._limit is not Undef: # XXX UNTESTED!
                self._select.limit = self._limit
            if self._offset is not Undef: # XXX UNTESTED!
                self._select.offset = self._offset
            return self._select

But it doesn't work in this case. This code only overrides .limit or .offset if the new value is not Undef. So if I slice the list again:

>>> tmp2 = foo[1:6]

The old .limit '2' will be replaced by '5' and I will get the correct slice. But when I call len(), the new limit is Undef, so the old value for .limit is left in place, and I get the FeatureError.

There are two solutions. Either change _get_select() to override a non-Undef value with Undef, or change the copy() operation to copy ._select.

It looks like this bug was fixed in Storm 0.17?

Never mind, it's not fixed on 0.17. I was looking at a hacked version of 0.17 on my own computer. This bug is still a big problem. I do have a workaround which I will apply to Launchpad.

This is biting us again on bug 627442. If fixing the problem is not a lot more work for us than continuing to work around it, we'd be interested. Would someone offer some guidance, per Leonard's comments? Thank you!

<niemeyer> lifeless: I see
<niemeyer> lifeless: Looks like an obvious bug indeed
<lifeless> are the solutions discussed in the bug appropriate ?
<niemeyer> lifeless: Probably not entirely
<niemeyer> lifeless: Discussing:
<niemeyer> """
<niemeyer> There are two solutions. Either change _get_select() to override a non-Undef value with Undef, or change the copy() operation to copy ._select.
<niemeyer> """
<niemeyer> The later probably means change copy() to *not* copy select (since it's already copying it nowadays)
<niemeyer> But it'd be wrong even then
<niemeyer> _select in this case is used exclusively on set operations
<niemeyer> So if it's not copied, the copy becomes incorrect immediately
<niemeyer> Turning _select into Undef would have the same result
<niemeyer> lifeless: That additional complexity is used specifically by _set_expr()
<niemeyer> lifeless: The right fix is likely in the direction of copying the outer expression contained in _select during copy(), or something similar (and less hackish)

I have written a test for the problem, and a possible fix: http://pastebin.ubuntu.com/486640/

I may not get all the implications, but it seems like a reasonable change.

The test and code in the paste look good to me.