Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7155 closed (fixed)

dates() querysets don't work after qsrf

Reported by: fcaprioli@… Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup post-qsrf
Cc: eric@…, jdunck@…, jarek.zgoda@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by ramiro)

After queryset refactor merge, a .dates() DateQueryset won't work on a null=True DateField.

During query setup, the queryset adds a .filter(fieldname__isnull=True) instead of .filter(fieldname__isnull=False) to avoid parsing null dates, so the resulting sql returns an empty list (or raise errors if there actually are null fields in the db).

I've attached a patch that fixes the problem

Attachments (2)

null_dates.patch (592 bytes) - added by fcaprioli@… 6 years ago.
dates-qsrf-7155.diff (2.1 KB) - added by jdunck 6 years ago.
Updated patch with regression test included.

Download all attachments as: .zip

Change History (18)

Changed 6 years ago by fcaprioli@…

comment:1 Changed 6 years ago by oyvind

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Spread across 3 tickets #7145, #7147

comment:2 Changed 6 years ago by Alex

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

I'm marking this as accepted, and the patch makes sense, however it would be nice if we could add a testcase for this regression.

comment:3 Changed 6 years ago by Eric Walstad <eric@…>

  • Cc eric@… added

comment:4 Changed 6 years ago by anonymous

  • Cc jdunck@… added

Changed 6 years ago by jdunck

Updated patch with regression test included.

comment:5 Changed 6 years ago by jdunck

  • Needs tests unset
  • Owner changed from nobody to jdunck
  • Status changed from new to assigned

comment:6 Changed 6 years ago by jdunck

  • Owner changed from jdunck to mtredinnick
  • Status changed from assigned to new

comment:7 Changed 6 years ago by jdunck

  • Owner changed from mtredinnick to nobody

comment:8 Changed 6 years ago by fcaprioli@…

  • Triage Stage changed from Accepted to Ready for checkin

I've been using the patched code in a fairly complex app (production server, a few million pages etc) without any problem; now that it includes tests (thank you jeremy!), this can be probably checked in even before malcolm's return. Any core dev interested? It is, after all, an harmless on-liner :)

comment:9 Changed 6 years ago by gav

  • Keywords qsrf-cleanup added
  • Triage Stage changed from Ready for checkin to Accepted

Non-core devs should not mark things as ready for checkin.

comment:10 Changed 6 years ago by Karen Tracey <kmtracey@…>

#7199 marked as a dup of this.

comment:11 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Triage Stage changed from Accepted to Ready for checkin

Bumping back up to ready for checkin based on Russell's note to django-dev:

http://groups.google.com/group/django-developers/msg/e8762784813b3921

where he says "If a ticket describes something that is clearly a bug, move to accepted (or ready for checkin if it's a trivial problem with a patch and tests ready to go)."

I think this one fits the "trivial" category and it has a test. It looks like at least 4 people have hit this bug and reported it in the tracker so it would be nice to have the very minor and obvious fix get into the code base.

comment:12 Changed 6 years ago by ramiro

  • Description modified (diff)

comment:13 Changed 6 years ago by jacob

  • milestone set to 1.0

comment:14 Changed 6 years ago by zgoda

  • Cc jarek.zgoda@… added

comment:15 Changed 6 years ago by russellm

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

(In [7739]) Fixed #7155 -- Corrected DateQuerySet to handle nullable fields. Thanks to fcaprioli@… for the original report and patch, and to Jeremy Dunck for the test case.

comment:16 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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.