[Django] #20393: django.db.models.query.QuerySet.__repr_

Django

unread,

May 10, 2013, 6:55:15 PM5/10/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Keywords: QuerySet, repr, side-
Severity: Normal | effect
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In trying to track down some timeouts and strange queries in our apps we
saw some strange queries being run by django. We were querying for a
single record with a FOR UPDATE, getting an error, then django would run
the same query 3 times but with a limit of 21 and then fetch all of the
records, each time.

Eventually I tracked this down to our error-handling middleware calling
repr() on a QuerySet, which then queried the database. `__repr__` is
supposed to be a string-representation of an object in its current state
and should not gave side-effects like making database queries, especially
in the case where the query uses FOR UPDATE or similar logic.

I suggest changing `django.db.models.query.QuerySet.__repr__` to only
touch the cached records and not use the local iterator, which can and
will query the DB, potentially causing issues like this.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

May 11, 2013, 9:10:51 PM5/11/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4

* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0

Comment:

Patch in pull request

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:1>

Django

unread,

May 12, 2013, 2:12:03 PM5/12/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4

Comment (by claudep):

Which pull request?

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:2>

Django

unread,

May 12, 2013, 5:34:03 PM5/12/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4

Comment (by lukeplant):

The reason for the current behaviour is to help interactive coding and
learning of Django - if you type an expression that returns a `QuerySet`,
you get something helpful. See the tutorial for example:

https://docs.djangoproject.com/en/1.5/intro/tutorial01/

This would have to be considered when weighing up this ticket - a lot of
Django tutorials (our own and others) would need significant modification,
and the habits of people used to the current behaviour.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:3>

Django

unread,

May 12, 2013, 6:16:44 PM5/12/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4

Comment (by anonymous):

pull request: https://github.com/django/django/pull/1055

I realize that it may inconvenience simple usage, but functions like
__str__ and __repr__ really should not be causing side effects in general.

In particular, debuggers and error reporting can cause strange side-
effects and errors. In our application, we've implemented a mysql
connection pool in the db backend. This works fine until an error happens
and the error-catching middleware we're using (sentry/raven) starts
accessing these query objects after the connection has been returned to
the pool.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:4>

Django

unread,

May 14, 2013, 10:16:34 AM5/14/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Type: Uncategorized | Status: closed

Component: Database layer | Version: 1.4

(models, ORM) | Resolution: wontfix

Changes (by lukeplant):

* status: new => closed
* resolution: => wontfix

Comment:

Well, you can argue that semantically these methods don't have side
effects. They do 'SELECT', not 'UPDATE'. They do happen to use IO, and can
therefore fail in all kinds of ways. "SELECT FOR UPDATE" is an unfortunate
edge case.

Looking at these docs:

[http://docs.python.org/2/reference/datamodel.html#object.__repr__]

... you have a conflict between the goal of being information rich and
with **always** being useful for debugging. In general, automatically
seeing the results is information rich and is useful in debugging, but
sometimes it causes further problems.

I have thought about it, and with this conflict of goals, I think we are
going to have to err on the side of the current behaviour. It is possible
to work around by not calling `repr` on `QuerySets` in your middleware,
and there are too many people who will be upset (or have problems with
their own debugging facilities) by changing this.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:5>

Django

unread,

May 15, 2013, 9:32:38 AM5/15/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Type: Uncategorized | Status: closed

Component: Database layer | Version: 1.4

Comment (by lukeplant):

I should have added - feel free to persuade us otherwise on the
DevelopersMailingList

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:6>

Django

unread,

May 24, 2013, 6:17:16 AM5/24/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Type: Uncategorized | Status: closed

Component: Database layer | Version: 1.4

Comment (by patrys):

What if we changed `__repr__` to be safe (only print query params) and
allowed a slice with `Ellipsis` to return a list of first N elements,
possibly executing the query if not evaluated earlier?

Example:

{{{
>>> x = Foo.objects.filter(age__gte=5)
<QuerySet(Foo, …)>
>>> x[...]
[<Foo>, <Foo>, <Foo>, <Foo>, '11 more...']
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:7>

Django

unread,

May 28, 2013, 8:17:13 PM5/28/13

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Type: Uncategorized | Status: closed

Component: Database layer | Version: 1.4

Comment (by anonymous):

That sounds like a great change to me.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:8>

Django

unread,

Oct 10, 2019, 11:54:44 AM10/10/19

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Type: Uncategorized | Status: closed

Component: Database layer | Version: 1.4
(models, ORM) |

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Matt Johnson):

I opened a discussion on the dev mailing list about this:

https://groups.google.com/forum/#!topic/django-developers/WzRZ0IfBipc

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:9>

Django

unread,

Oct 11, 2019, 10:29:25 PM10/11/19

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |

Severity: Normal | Resolution:

Keywords: QuerySet, repr, | Triage Stage:
side-effect | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Matt Johnson):

* status: closed => new
* resolution: wontfix =>

Comment:

Re-opening based on the mailing list discussions.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:10>

Django

unread,

Oct 12, 2019, 12:01:10 PM10/12/19

to django-...@googlegroups.com

#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-------------------------------------+-------------------------------------
Reporter: justin@… | Owner: nobody

Keywords: QuerySet, repr, | Triage Stage:
side-effect | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: new => closed

* type: Uncategorized => Cleanup/optimization
* version: 1.4 => master
* resolution: => wontfix

Comment:

2 days of discussion doesn't look sufficient for reopening ticket that was
closed 6 years ago and is also backward incompatible. We have basically
only 2 responses, let's wait for a strong consensus.

--
Ticket URL: <https://code.djangoproject.com/ticket/20393#comment:11>

Reply all

Reply to author

Forward

[Django] #20393: django.db.models.query.QuerySet.__repr__ should not have side-effects

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

[Django] #20393: django.db.models.query.QuerySet.repr should not have side-effects