Opened 8 years ago

Last modified 4 years ago

#26434 new Bug

Inconsistent results of QuerySet count() method using PostgreSQL backend prior and post the QuerySet evaluation

Reported by: kamandol Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: postgresql queryset count annotate aggreagate order_by
Cc: github@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Using a PostgreSQL backend database, in some cases with QuerySets involving annotations, values or values_list and ordering(order_by) clauses the resulting QuerySet count() method fails to predict the real row result unless that QuerySet is evaluated.

For instance:

q = BundleVersion.objects.order_by('-id').values_list("port__id", "asset_bundle__id").annotate(max=models.Max("id"))

This QuerySet using PostgreSQL would, in fact, group the results by BundleVersion.id instead of the tuple ("port__id", "asset_bundle__id") due to the order_by clause using a column not in the values_list. This is a documented behavior.

The problem is that if q is not evaluated yet, calling q.count() will return an amount of rows as if the grouping was done on the tuple ("port__id", "asset_bundle__id"). After evaluating q, or calling len(q), the result of q.count() will be the expected, as if the grouping was done using 'BundleVersion.id'.

Change History (13)

comment:1 by Tim Graham, 8 years ago

Could you provide a test for Django's test suite in tests/queries/tests.py that demonstrates the issue?

in reply to:  1 comment:2 by kamandol, 8 years ago

Replying to timgraham:

Could you provide a test for Django's test suite in tests/queries/tests.py that demonstrates the issue?

Here is the commit with the demonstration:

https://github.com/sp-ricard-valverde/django/commit/c50afafdb02880e4c941c2a72a215bee80de3aed

comment:3 by Iacopo Spalletti, 8 years ago

Tests looks legitimate, but I don't get this one https://github.com/sp-ricard-valverde/django/commit/c50afafdb02880e4c941c2a72a215bee80de3aed#diff-ce9b52a66d03e851a9828377263dc04bR3864 ; or rather I don't get why is labeled as 'OK' as it the concatenation of the previous ones

comment:4 by Iacopo Spalletti, 8 years ago

Cc: github@… added

in reply to:  3 comment:5 by kamandol, 8 years ago

Replying to yakky:

Tests looks legitimate, but I don't get this one https://github.com/sp-ricard-valverde/django/commit/c50afafdb02880e4c941c2a72a215bee80de3aed#diff-ce9b52a66d03e851a9828377263dc04bR3864 ; or rather I don't get why is labeled as 'OK' as it the concatenation of the previous ones

Sorry, my bad, it's a bad choice for a test title. What it demonstrates is that the second assert in that test is successful even though a new QuerySet is built and, according to the other failed tests it should fail too.

comment:6 by kamandol, 8 years ago

Last edited 8 years ago by kamandol (previous) (diff)

comment:7 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

If you could put the tests on a branch of you fork other than "master" that seems a bit safer. There is probably no need for skipUnlessDBEngine - is there a reason the queries shouldn't also work on other databases? Anyway, the usual way to skip tests is based on database features.

comment:8 by kamandol, 8 years ago

I will change the branch as requested.

You're right, the faulty behavior was detected on PostgreSQL, but on MySQL worked correctly so the test should pass. Could not test other db engines though. I will remove that extra piece of code.

comment:9 by Can Sarıgöl, 5 years ago

Has patch: set
Last edited 5 years ago by Can Sarıgöl (previous) (diff)

comment:10 by Mariusz Felisiak, 5 years ago

Has patch: unset
Version: 1.9master

comment:11 by Can Sarıgöl, 5 years ago

Last edited 5 years ago by Can Sarıgöl (previous) (diff)

comment:12 by Can Sarıgöl, 5 years ago

Has patch: set

comment:13 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top