Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15999 closed Bug (invalid)

Django admin count is run on different query than listed rows

Reported by: craig.kerstiens@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

Within django admin, when count is run it includes the filters but omits the joins. In some cases this can cause the count to return an incorrect number from what is displayed in the list. The count should be run across the same query that returns the results to the admin list.

Attachments (1)

sampleapp.tgz (10.0 KB) - added by Craig Kerstiens <craig.kerstiens@…> 3 years ago.
Example repro app

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by lukeplant

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

This report is rather vauge. There are filters and joins that can be added in various different ways - and in fact filters often require joins to be added. There are also several 'counts' that appear on an admin change list, depending on paging and filters!

Please give a fuller report, preferably with an example, so we can track down what you mean.

It is possible, given that the admin uses django.core.paginator.Paginator, that it is Paginator at fault, or the ORMs QuerySet.count() method, but it is very difficult to tell with this report.

comment:2 Changed 3 years ago by Craig Kerstiens <craig.kerstiens@…>

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

The simplest way to reproduce this is to include a foreign key within your list display. When you have the foreign key there, select related is called, if you have another foreign key (not displayed in the list), that is not set to Null=True, it runs an inner join, something similar to:

SELECT table1.col1, table2.col2
FROM table1
INNER JOIN table2 on table1.col1 = table2.col1
INNER JOIN table3 on table1.col1 = table3.col1

The resulting query produces less results than a query similar to:

SELECT COUNT(*) FROM table_foo WHERE column_name = "bar"

One would expect for the count to be run on top of the initial query that produces the results (with all filters or joins on it).

comment:3 Changed 3 years ago by Craig Kerstiens <craig.kerstiens@…>

  • Component changed from contrib.admin to Core (Other)

Upon further investigation the count on the query is correct, it is specifically with how paginator runs count that it returns incorrectly.

comment:4 Changed 3 years ago by lukeplant

  • Resolution set to needsinfo
  • Status changed from reopened to closed

It is difficult to work out how Paginator could be at fault, since to get the count it simply does QuerySet.count(). So that must be the method at fault, which takes us to django/db/models/sql/query/Query.get_count(), which is not removing any filters or joins as far as I can see.

And we still don't have enough information to reproduce this - an example set of models, and some example code using Paginator or QuerySet.count() is what we need. If it is the latter at fault, we simply need an example where len(queryset) != queryset.count().

Please re-open if you can provide that. Thanks!

Changed 3 years ago by Craig Kerstiens <craig.kerstiens@…>

Example repro app

comment:5 Changed 3 years ago by Craig Kerstiens <craig.kerstiens@…>

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

I've attached an example app that reproduces the case. Included is a db with data that does reproduce it. If you navigate within the admin to models1, you'll see the count lists 101 items, where the list shows none, due to the inner join canceling out items.

Username/pass for admin is: demo/demo,

You should be able to boot strap the entire example app with:

virtualenv --no-site-packages .
source bin/activate
bin/pip install -r requirements.txt

comment:6 Changed 3 years ago by lukeplant

  • Component changed from Core (Other) to Database layer (models, ORM)
  • Triage Stage changed from Unreviewed to Accepted

Many thanks for taking the time to do this, I can reproduce it now.

We've got at least 2 potential bugs:

1) The fact that appending select_related() to Model1.objects.all() causes the query to return no objects. Perhaps we need to special foreign keys to self somehow, not sure.

2) The fact that Model1.objects.all().select_related().count() != len(Model1.objects.all().select_related())

comment:7 Changed 3 years ago by lukeplant

  • Resolution set to invalid
  • Status changed from reopened to closed

OK, cancel that. I've found your table contains invalid data - the 'self_ref_id' column of the Model1 is full of NULLs, but the DB model definition doesn't allow that. If I put null=True i.e. self_ref = models.ForeignKey('self', null=True), then everything is fine again.

Please re-open if this doesn't fix your issue.

comment:8 Changed 3 years ago by julien

This might be a similar "issue" as in #11460.

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.