Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#13282 closed (fixed)

Document patch: WEEK_DAY should start from 0, not 1

Reported by: wangchun Owned by: nobody
Component: Documentation Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Confirmed with postgresql

Attachments (3)

13282.diff (1015 bytes) - added by wangchun 7 years ago.
13282-r13135.diff (939 bytes) - added by Ramiro Morales 7 years ago.
13282-r13135.2.diff (939 bytes) - added by Tim Graham 7 years ago.
ramiro's patch with # for Saturday fixed from 6 to 7

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by wangchun

Attachment: 13282.diff added

comment:1 Changed 7 years ago by Karen Tracey

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

There is a slight problem in the doc here, but the patch does not fix it. The only thing wrong in the cited doc is that the "SQL equivalent" is not in fact the SQL produced, for any supported database, for the given example. For the function as implemented, the days of the week do start on Sunday and the numbering does starts at 1, as documented. So the parts of the patch which change that are incorrect. The example query, using week_day of 2, will in fact produce all matching objects that fall on a Monday.

The problem in the doc is that the extract(dow) function is what is used to implement the feature on PostgreSQL, and there the days are numbered differently. If you check the actual SQL generated for the example query, you will see that it adds 1 to the result of extract(dow) and compares against the requested numerical day of week. Putting the actual PostgreSQL query in the doc, would, I think, be more confusing than helpful. Putting the MySQL version of the query (DAYOFWEEK(pub_date) = 2) would likely be more helpful. Alternatively we could remove the "SQL equivalent" entirely and just describe what the function does without including any indication of the SQL generated (which in fact is very different for the different databases).

Note changing the numbering is not an option, this was already requested and rejected in #10345, and cannot be changed at this point since it would break backwards compatibility.

Changed 7 years ago by Ramiro Morales

Attachment: 13282-r13135.diff added

comment:2 in reply to:  1 Changed 7 years ago by Ramiro Morales

Has patch: set
milestone: 1.2

Replying to kmtracey:

Alternatively we could remove the "SQL equivalent" entirely and just describe what the function does without including any indication of the SQL generated (which in fact is very different for the different databases).

I've attached a patch implementing just that choice as outlined by Karen.

Changed 7 years ago by Tim Graham

Attachment: 13282-r13135.2.diff added

ramiro's patch with # for Saturday fixed from 6 to 7

comment:3 Changed 7 years ago by Tim Graham

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:4 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [13155]) Fixed #13282 -- Clarified documentation around week_day filtering in querysets. Thanks to wangchun, Ramiro Morales and timo.

comment:5 Changed 7 years ago by Russell Keith-Magee

(In [13159]) [1.1.X] Fixed #13282 -- Clarified documentation around week_day filtering in querysets. Thanks to wangchun, Ramiro Morales and timo.

Backport of r13155 from trunk.

comment:6 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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