Opened 6 years ago

Closed 20 months ago

Last modified 7 months ago

#11629 closed Cleanup/optimization (fixed)

Deprecate callable QuerySet filter arguments

Reported by: harrym Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Apologies if I've missed this somewhere, but I couldn't find anything in the documentation that states that the keyword arguments passed to a QuerySet's filter method may be callable. This is useful in cases such as when QuerySets are constructed for generic views, and you want to filter by a time relative to now. This appears in the code in django/db/models/sql/query.py, line 1546.

Change History (12)

comment:1 Changed 6 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:3 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:4 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:5 Changed 2 years ago by bmispelon

  • Cc bmispelon@… added

The code has moved around a bit since the report but the feature still exists: https://github.com/django/django/blob/f403653cf146384946e5c879ad2a351768ebc226/django/db/models/sql/query.py#L1074

It's still not documented anywhere that I could find but more interestingly, I couldn't find any tests for it either.

Note that the feature doesn't only apply to Queryset.filter but also to Queryset.exclude, Queryset.get, and Q objects.

Interestingly, it also sorta works for get_or_create but only for the get part. If the object doesn't exist in the database, then django will set the model field's value to the callable and things will most likely break when attempting to save to the database.

I think a good place to document this would be in the "Field lookups" paragraph of the queryset API reference page: https://docs.djangoproject.com/en/dev/ref/models/querysets/#id4.

comment:6 Changed 2 years ago by manojlds@…

I see it mentioned in the tutorials - https://docs.djangoproject.com/en/dev/intro/tutorial05/

Notice that we use a callable queryset argument, timezone.now, which will be evaluated at request time. If we had included the parentheses, timezone.now() would be evaluated just once when the web server is started.

comment:7 Changed 2 years ago by bmispelon

It looks like the feature is "broken" since callable arguments are evaluated when the queryset is built, not when it's evaluated.

This means (if I understand it right), that doing queryset.filter(foo=some_callable) is basically the same as doing queryset.filter(foo=some_callable()), which makes this feature less useful.

This was discovered in ticket #20241.

I think the feature would be useful but after discussing it on IRC, it seems it might be quite tricky to get it to work.

comment:8 Changed 20 months ago by bmispelon

Another ticket linked with this issue: #21632.

comment:9 Changed 20 months ago by bmispelon

  • Has patch set

On top of being undocumented, that feature is also untested (full test suite still passed after removing the two lines that call the value if it's callable).

Plus, as mentionned on #20241, this feature is going to be tricky to implement in a useful way.

Consequently, I propose to remove the feature altogether (going through the usual deprecation path).

Here's a PR with docs and some tests: https://github.com/django/django/pull/2091

comment:10 Changed 20 months ago by timo

  • Component changed from Documentation to Database layer (models, ORM)
  • Summary changed from Callable QuerySet filter arguments not mentioned in documentation to Deprecate callable QuerySet filter arguments
  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 20 months ago by Baptiste Mispelon <bmispelon@…>

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

In f1b3ab9c2158f5a7da113aef4158499ce2d42ee2:

Fixed #11629 -- Deprecated callable arguments to queryset methods.

Callable arguments were an untested and undocumented feature.

comment:12 Changed 7 months ago by Tim Graham <timograham@…>

In 2cd00424c4a14b04973629d9f027df5281806e15:

Removed support for callable QuerySet arguments per deprecation timeline; refs #11629.

Note: See TracTickets for help on using tickets.
Back to Top