From 5da47e43c7cb051991a3248311248cf6ba7ff27d Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Mon, 30 Jun 2008 06:24:21 +0000 Subject: [PATCH] Fixed #7314 -- Changed the way extra() bits are handled when QuerySets are merged. Also added a section to the documentation to indicate why it's probably not a good idea to rely on this feature for complex stuff. Garbage in, garbage out applies even to Django code. Thanks to erik for the test case for this one. git-svn-id: http://code.djangoproject.com/svn/django/trunk@7791 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/sql/query.py | 19 +++++-- docs/db-api.txt | 25 +++++++++ .../regressiontests/extra_regress/__init__.py | 0 tests/regressiontests/extra_regress/models.py | 55 +++++++++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 tests/regressiontests/extra_regress/__init__.py create mode 100644 tests/regressiontests/extra_regress/models.py diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 764a256006..6c06609969 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -366,10 +366,21 @@ class Query(object): item.relabel_aliases(change_map) self.select.append(item) self.select_fields = rhs.select_fields[:] - self.extra_select = rhs.extra_select.copy() - self.extra_tables = rhs.extra_tables - self.extra_where = rhs.extra_where - self.extra_params = rhs.extra_params + + if connector == OR: + # It would be nice to be able to handle this, but the queries don't + # really make sense (or return consistent value sets). Not worth + # the extra complexity when you can write a real query instead. + if self.extra_select and rhs.extra_select: + raise ValueError("When merging querysets using 'or', you " + "cannot have extra(select=...) on both sides.") + if self.extra_where and rhs.extra_where: + raise ValueError("When merging querysets using 'or', you " + "cannot have extra(where=...) on both sides.") + self.extra_select.update(rhs.extra_select) + self.extra_tables += rhs.extra_tables + self.extra_where += rhs.extra_where + self.extra_params += rhs.extra_params # Ordering uses the 'rhs' ordering, unless it has none, in which case # the current ordering is used. diff --git a/docs/db-api.txt b/docs/db-api.txt index f80d63797a..9a604bf320 100644 --- a/docs/db-api.txt +++ b/docs/db-api.txt @@ -443,6 +443,31 @@ This is roughly equivalent to:: Note, however, that the first of these will raise ``IndexError`` while the second will raise ``DoesNotExist`` if no objects match the given criteria. +Combining QuerySets +------------------- + +If you have two ``QuerySet`` instances that act on the same model, you can +combine them using ``&`` and ``|`` to get the items that are in both result +sets or in either results set, respectively. For example:: + + Entry.objects.filter(pubdate__gte=date1) & \ + Entry.objects.filter(headline__startswith="What") + +will combine the two queries into a single SQL query. Of course, in this case +you could have achieved the same result using multiple filters on the same +``QuerySet``, but sometimes the ability to combine individual ``QuerySet`` +instance is useful. + +Be careful, if you are using ``extra()`` to add custom handling to your +``QuerySet`` however. All the ``extra()`` components are merged and the result +may or may not make sense. If you are using custom SQL fragments in your +``extra()`` calls, Django will not inspect these fragments to see if they need +to be rewritten because of changes in the merged query. So test the effects +carefully. Also realise that if you are combining two ``QuerySets`` with +``|``, you cannot use ``extra(select=...)`` or ``extra(where=...)`` on *both* +``QuerySets``. You can only use those calls on one or the other (Django will +raise a ``ValueError`` if you try to use this incorrectly). + QuerySet methods that return new QuerySets ------------------------------------------ diff --git a/tests/regressiontests/extra_regress/__init__.py b/tests/regressiontests/extra_regress/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/extra_regress/models.py b/tests/regressiontests/extra_regress/models.py new file mode 100644 index 0000000000..3bcc3075ec --- /dev/null +++ b/tests/regressiontests/extra_regress/models.py @@ -0,0 +1,55 @@ +import copy + +from django.db import models +from django.db.models.query import Q + + +class RevisionableModel(models.Model): + base = models.ForeignKey('self', null=True) + title = models.CharField(blank=True, max_length=255) + + def __unicode__(self): + return u"%s (%s, %s)" % (self.title, self.id, self.base.id) + + def save(self): + super(RevisionableModel, self).save() + if not self.base: + self.base = self + super(RevisionableModel, self).save() + + def new_revision(self): + new_revision = copy.copy(self) + new_revision.pk = None + return new_revision + +__test__ = {"API_TESTS": """ +### Regression tests for #7314 and #7372 + +>>> rm = RevisionableModel.objects.create(title='First Revision') +>>> rm.pk, rm.base.pk +(1, 1) + +>>> rm2 = rm.new_revision() +>>> rm2.title = "Second Revision" +>>> rm2.save() +>>> print u"%s of %s" % (rm2.title, rm2.base.title) +Second Revision of First Revision + +>>> rm2.pk, rm2.base.pk +(2, 1) + +Queryset to match most recent revision: +>>> qs = RevisionableModel.objects.extra(where=["%(table)s.id IN (SELECT MAX(rev.id) FROM %(table)s AS rev GROUP BY rev.base_id)" % {'table': RevisionableModel._meta.db_table,}],) +>>> qs +[] + +Queryset to search for string in title: +>>> qs2 = RevisionableModel.objects.filter(title__contains="Revision") +>>> qs2 +[, ] + +Following queryset should return the most recent revision: +>>> qs & qs2 +[] + +"""}