QuerySets that use F() + timedelta() crash when compiling their query more than once

[Tim Graham]

Could you write a regression test that demonstrates the problem and proves it's fixed?

[despawn@…]

Regression test.

[Tim Graham]

Verified the test and the rest of the test suite on SQlite.

[Anssi Kääriäinen]

Wouldn't it be better to use just node.children[-1] instead of .pop()?

[despawn@…]

I've thought about that, but then the datetime value gets put in the query the second time (and incorrectly) in evaluate_node. There's probably a better way to do this, but I'm not exactly knowledgeable enough about Django internals to figure it out. The patch attached works as a quick-and-easy fix.

[Alexey Voronov]

Sorry, I meant the timedelta value.

[Anssi Kääriäinen]

OK, good enough for me. Is this a regression in 1.6 or has this bug existed for a longer time (that is, does this need to be backpatched to 1.6)?

[Alexey Voronov]

Google seems to find reports of the error from way back in December 2011. The code in question hasn't been touched since it appeared with the introduction of support for F() + timedelta(). Looks like it's been this way for years.

[Anssi Kääriäinen <akaariai@…>]

In 7f2485b4d180956aa556a88ed1d9754eeeeea054:

Fixed #21643 -- repeated execution of qs with F() + timedelta

Thanks Tim Graham for review.

[Tai Lee]

This is a regression in 1.6, I believe due to the deep copy changes when cloning querysets. I confirmed that F() + timedelta() does work in 1.5.

Previously, the DateModifierNode expression would have been deep copied every time a queryset was cloned, so it would never have been evaluated twice.

So I think this should be backported.

Also #22101 was a duplicate.

[Tai Lee]

The test from the fix already applied also fails in 1.5, but the following test passes in 1.5 and fails in 1.6 showing that this is a regression.

    def test_query_clone(self):
        # Ticket #21643
        qs = Experiment.objects.filter(end__lt=F('start') + datetime.timedelta(hours=1))
        qs2 = qs.all()
        list(qs)
        list(qs2)

Is it enough to back port the existing commit, or should this new test also be committed and back ported?

[Tim Graham <timograham@…>]

In 8137215973c8cf97f58f244021b1a4e75923ade8:

Added release note and regression test for refs #21643.

This will be backported to stable/1.6.x along with the original fix.

[Tim Graham <timograham@…>]

In 5cda1d27027ea74d8a1b53e43bef697cd4426e9a:

[1.6.x] Fixed #21643 -- repeated execution of qs with F() + timedelta

Thanks Tim Graham for review and Tai Lee for the additional test to prove
this was a regression in 1.6.

Backport of 7f2485b4d1 and 8137215973 from master