From faf6a911ad7357c8dd1defa3be0317a8418febf7 Mon Sep 17 00:00:00 2001 From: Artur Frysiak Date: Sat, 15 Feb 2014 22:44:14 +0100 Subject: [PATCH] Fixed #22023 -- Raised an error for values() followed by defer() or only(). Previously, doing so resulted in invalid data or crash. Thanks jtiai for the report and Karol Jochelson, Jakub Nowak, Loic Bistuer, and Baptiste Mispelon for reviews. --- django/db/models/query.py | 6 ++++++ docs/ref/models/querysets.txt | 15 ++++++++++++--- docs/releases/1.7.txt | 6 ++++++ tests/queries/tests.py | 10 ++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 725c04caca..4d258ff60d 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1060,6 +1060,12 @@ class ValuesQuerySet(QuerySet): # QuerySet.clone() will also set up the _fields attribute with the # names of the model fields to select. + def only(self, *fields): + raise NotImplementedError("ValuesQuerySet does not implement only()") + + def defer(self, *fields): + raise NotImplementedError("ValuesQuerySet does not implement defer()") + def iterator(self): # Purge any extra columns that haven't been explicitly asked for extra_names = list(self.query.extra_select) diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 05f7d36aa6..339b5e18fe 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -502,14 +502,23 @@ A few subtleties that are worth mentioning: made after a ``values()`` call will have its extra selected fields ignored. +* Calling :meth:`only()` and :meth:`defer()` after ``values()`` doesn't make + sense, so doing so will raise a ``NotImplementedError``. + +.. versionadded:: 1.7 + + The last point above is new. Previously, calling :meth:`only()` and + :meth:`defer()` after ``values()`` was allowed, but it either crashed or + returned incorrect results. + A ``ValuesQuerySet`` is useful when you know you're only going to need values from a small number of the available fields and you won't need the functionality of a model instance object. It's more efficient to select only the fields you need to use. -Finally, note a ``ValuesQuerySet`` is a subclass of ``QuerySet``, so it has all -methods of ``QuerySet``. You can call ``filter()`` on it, or ``order_by()``, or -whatever. Yes, that means these two calls are identical:: +Finally, note that a ``ValuesQuerySet`` is a subclass of ``QuerySet`` and it +implements most of the same methods. You can call ``filter()`` on it, +``order_by()``, etc. That means that these two calls are identical:: Blog.objects.values().order_by('id') Blog.objects.order_by('id').values() diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 8e241caf44..b2be67d390 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -616,6 +616,12 @@ Models :attr:`~django.db.models.ForeignKey.limit_choices_to` when defining a ``ForeignKey`` or ``ManyToManyField``. +* Calling :meth:`only() ` and + :meth:`defer() ` on the result of + :meth:`QuerySet.values() ` now raises + an error (before that, it would either result in a database error or + incorrect data). + Signals ^^^^^^^ diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 3f861db5d1..7bb5bb34d0 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -1275,6 +1275,16 @@ class Queries3Tests(BaseQuerysetTest): Item.objects.datetimes, 'name', 'month' ) + def test_ticket22023(self): + # only() and defer() are not applicable for ValuesQuerySet + with self.assertRaisesMessage(NotImplementedError, + "ValuesQuerySet does not implement only()"): + Valid.objects.values().only() + + with self.assertRaisesMessage(NotImplementedError, + "ValuesQuerySet does not implement defer()"): + Valid.objects.values().defer() + class Queries4Tests(BaseQuerysetTest): def setUp(self):