Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#13099 closed (fixed)

Documentation: Second exclude example is incorrect

Reported by: istruble Owned by: nobody
Component: Documentation Version: 1.2-beta
Severity: Keywords:
Cc: istruble@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

See http://docs.djangoproject.com/en/dev/ref/models/querysets/#exclude-kwargs.

The second example attempts to show how to create an ORed query but creates an ANDed query.

This example has been worked on a few times but is still in need of attention. Ticket #10631 requested that the example's SQL be changed. The ticket was rejected because the example SQL did in fact match up with the example code at the time. However, the code did not match up with the description. This code and description mismatch was not mentioned in the ticket.

This example excludes all entries whose ``pub_date`` is later than 2005-1-3 OR whose headline is "Hello"::
 
    Entry.objects.exclude(pub_date__gt=datetime.date(2005, 1, 3)).exclude(headline='Hello')

In SQL terms, that evaluates to::

    SELECT ...
    WHERE NOT pub_date > '2005-1-3'
    AND NOT headline = 'Hello'

Note the second example is more restrictive.

The SQL was updated with commit #10303 to match the example's "OR" description. This left a mismatch between the code and description.

    SELECT ...
    WHERE NOT pub_date > '2005-1-3'
    OR NOT headline = 'Hello'

This example should either have it's example code be updated so that the description, code and SQL all match up or the entire example should be replaced with a reference to the complex lookups with Q. I think it would be best to replace the example with a reference to the complex lookups with Q. Someone else may have a good reason for updating it instead of replacing it so here is an update to the example code:

    Entry.objects.exclude(pub_date__gt=datetime.date(2005, 1, 3)) |
           Entry.objects.exclude(headline='Hello') 

Diffs for both replacing and updating the example are attached. Choose one.

Attachments (2)

ticket_13099_replace_example.diff (829 bytes) - added by istruble 5 years ago.
ticket_13099_update_example.diff (929 bytes) - added by istruble 5 years ago.

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by istruble

Changed 5 years ago by istruble

comment:1 Changed 5 years ago by istruble

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

comment:2 follow-up: Changed 5 years ago by russellm

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Ready for checkin

It's a bit hard to track exactly what sequence of events has happened here, but the SQL example should definitely read AND.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 5 years ago by istruble

Replying to russellm:

[...] the SQL example should definitely read AND.

It would be more correct to say that the current example code will generate SQL with an AND in it but this example needs code that will generate SQL with an OR in it.

I should have made it more clear that there are two examples given in this section of the documentation. The first example accurately shows exclusion with AND. The second example attempts to show exclusion with OR but the code portion of it needs attention. The intent of this ticket is to fix the second example in the section.

Erring on the side of caution, here is a rewritten problem statement.


Currently example 2 of the exclude(kwargs) section looks like this:

  • description-with-OR
  • code-with-AND-logic
  • sql-with-OR

We need to update the code so that example 2 looks like this:

  • description-with-OR
  • code-with-OR-logic
  • sql-with-OR

comment:4 in reply to: ↑ 3 Changed 5 years ago by lukeplant

Replying to istruble:

Replying to russellm:

[...] the SQL example should definitely read AND.

It would be more correct to say that the current example code will generate SQL with an AND in it but this example needs code that will generate SQL with an OR in it.

The example is to demonstrate the difference between use of a single call to .exclude() with two parameters, and multiple calls to .exclude(). It is in fact only the SQL that is incorrect - the English matches the code. If I "exclude" everything matching predicate_A OR predicate_B, then I "select" everything matching NOT predicate_A AND NOT predicate_B (De Morgan's laws).

comment:5 Changed 5 years ago by lukeplant

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

(In [12936]) Fixed #13099 - incorrect SQL for exclude() example

Thanks to istruble for the report.

comment:6 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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