Code

Opened 12 months ago

Closed 4 months ago

#20241 closed Bug (duplicate)

QuerySets with callable filter args have said args evaluated only once

Reported by: christopher.m.pike@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I was going to write out a longer ticket, but this SO post does a good job of describing my situation.

http://stackoverflow.com/questions/15666199/new-rows-created-in-django-admin-arent-displayed-on-site

I'm having the same issue, but I'm using Python 2.7.3 with PostgreSQL with Django 1.5.1.

Attachments (0)

Change History (6)

comment:1 Changed 12 months ago by bmispelon

  • Cc bmispelon@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Release blocker to Normal
  • Triage Stage changed from Unreviewed to Accepted

I can reproduce this issue.

Using the following code, you can see that the callable arguments are indeed only evaluated once (when the urls.py file is read for the first time):

from django.conf.urls import patterns, include, url
from django.views.generic import ListView
from django.utils import timezone

from .models import Poll


def foo():
    print('called')
    return timezone.now()


urlpatterns = patterns('',
    url('^$', ListView.as_view(
        queryset=Poll.objects.filter(pub_date__lte=foo),
    )),
)

However, unlike what it described on the linked SO post, I find that the issue is present on master, 1.5, and 1.4 so I don't think it's a regression.

This feature (passing callable arguments to filter) is only mentionned in the 5th part of the tutorial and it's not documented anywhere else (see #11629).

I'm not sure if this is a bug or intended behavior.

I the issue here lies with the documentation and I would suggest re-writing that part of the tutorial.

With normal pyhton code, anything sitting at the top-level of a module is evaluated when the file is read.
In the case of querysets, they do magic things to delay their evaluation which makes the code kind-of work but I don't think it's a good habit to teach.

comment:2 follow-up: Changed 12 months ago by michael.c.urbanski@…

I was under the impression that callables in QuerySets, like callables in fields, were called as they were used. Why pass in a callable if it's only evaluated once?

comment:3 in reply to: ↑ 2 Changed 12 months ago by bmispelon

Replying to michael.c.urbanski@…:

I was under the impression that callables in QuerySets, like callables in fields, were called as they were used.

I would have thought so too, but it appears it's not the case.

I've opened #20249 to fix the tutorial while this ticket remains open (fixing this ticket is likely more complicated than amending the documentation).

comment:4 Changed 12 months ago by akaariai

The reasons why fixing this isn't easy:

  1. The WhereNode._prepare_data() call is done at .filter() call time. The _prepare_data() method does some transformations based on the type of the value, specifically datetime.datetime values will cause some transformations. If the callable is timezone.now, then the lookup will not work correctly.
  2. The value will need to be resolved for isnull lookups (isnull=True lookups will need to produce LEFT OUTER JOINs and join promotion is done at .filter() call time).
  3. Some advanced constructs do not work correctly - returning F() objects for example (they don't work correctly even now).

At least no.1 will need to be fixed if the value resolving is deferred to query execution time. So, _prepare_data will need to be moved to query execution time. The only problematic case seems to be iterators - they must not be consumed multiple times, and this would happen if all of _prepare_data is moved to query execution time.

Fixing number 2 and 3 properly will be really hard (need to alter the join types at execution time, and that is problematic for multiple reasons, not least because the query should not be altered at execution time...). But, I am not sure if those need to be fixed at all, they could be documented as known limitations when using callable arguments. In practice the limitations wouldn't cause much trouble to users, and it is easy enough to raise errors in problematic cases.

comment:5 Changed 12 months ago by bmispelon

Turns out this issue was reported a while ago: #10466

I closed the other one since I think there's more information in the discussion of this ticket.

I don't know anything about the internals of querysets but if this issue is that tricky to fix, I'd be in favor of dropping it entirely.

For one, it's never been documented and secondly, it's not very useful since the callable is evaluated immediately (all it does is save the user from typing a pair of parentheses).

comment:6 Changed 4 months ago by timo

  • Resolution set to duplicate
  • Status changed from new to closed

We're going to deprecate callable queryset filter arguments -- see #11629.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.