Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13415 closed (duplicate)

Incorrect SQL boolean expression for multiple aggregate filters

Reported by: Beuc Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

See the test app attached (with test case).

The query:

        inactive_books_qs = Book.objects \
            .annotate(last_review_date=Max('review__date')) \
            .annotate(last_publication_date=Max('publication__date')) \
            .filter(last_publication_date__lt=tree_months_ago) \
            .filter(Q(last_review_date__lt=tree_months_ago)|Q(last_review_date__isnull=True)) \
            .only('id')

The resulting SQL:

SELECT [...] HAVING (MAX("aggregatebug_publication"."date") < 2010-01-21  AND MAX("aggregatebug_review"."date") < 2010-01-21  AND MAX("aggregatebug_review"."date") IS NULL)

The expected SQL:

SELECT [...] HAVING MAX("aggregatebug_publication"."date") < 2010-01-21  AND (MAX("aggregatebug_review"."date") < 2010-01-21  OR MAX("aggregatebug_review"."date") IS NULL)

Issues:

  • parenthesis not closed after MAX("aggregatebug_publication"."date") < 2010-01-21
  • AND instead of OR!!! MAX("aggregatebug_review"."date") < 2010-01-21 AND MAX("aggregatebug_review"."date") IS NULL
  • no parenthesis around the test above

There may be multiple bugs. When trying to fix things around:

Index: models/sql/query.py
===================================================================
--- models/sql/query.py	(révision 13022)
+++ models/sql/query.py	(copie de travail)
@@ -1013,7 +1013,7 @@
                 entry.add((aggregate, lookup_type, value), AND)
                 if negate:
                     entry.negate()
-                self.having.add(entry, AND)
+                self.having.add(entry, connector)
                 return
 
         opts = self.get_meta()

(this is by no way a suggested patch)

well in that case I got s/AND/OR/ correctly, but still the wrong parenthesis:

HAVING ((MAX("aggregatebug_publication"."date") < 2010-01-21  AND MAX("aggregatebug_review"."date") < 2010-01-21 ) OR MAX("aggregatebug_review"."date") IS NULL)

(should be around OR, not around AND)

Attachments (1)

bug.tar.gz (3.1 KB) - added by Beuc 4 years ago.
testcase for aggregation bug

Download all attachments as: .zip

Change History (4)

Changed 4 years ago by Beuc

testcase for aggregation bug

comment:1 Changed 4 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

This is a dupe of #11293.

comment:2 Changed 4 years ago by Beuc

Damn I missed the "ORM aggregation" component (looked in "Database layer") :(

comment:3 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 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.