Django

Code

Ticket #2076 (closed: fixed)

Opened 2 years ago

Last modified 2 weeks ago

order_by with related table does not work

Reported by: mtredinnick Assigned to: nobody
Component: Database wrapper Version: SVN
Keywords: qs-rf-fixed Cc: gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz, eliott@cactuswax.net, pythonmailing@web.de
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 1

Description

This came up on IRC today: if you look at the order_by query that uses the related table Blogs in http://www.djangoproject.com/documentation/db_api/#order-by-fields, it will not actually work. The SQL we produce does not join the blogs_blog table to the blogs_entry table in the SQL query, although it does put in an "order by blogs_blog.name ASC" bit. We need to add in the extra table to the join as well, although this requires reordering the execution in django/db/models/query.py (in the _get_sql_clause() method).

A temporary hack for the moment is Entry.objects.select_related().order_by('blogs_blog.name', 'headline') but that is a bit clunky.

We need a test for this, too.

(assigning to myself for now; just noting this here in case I get hit by a bus.)

Attachments

ticket-2076.diff (9.3 kB) - added by ramiro <rm0 _at_ gmx.net> on 02/02/07 10:23:02.
Patch originally posted by serbaut to #2210 with tests added
ticket-2076.2.diff (9.4 kB) - added by Michael Radziej <mir@noris.de> on 02/02/07 11:45:48.
slightly corrected version of previous patch (missing backticks)
ticket-2076.3.diff (8.9 kB) - added by mir@noris.de on 02/22/07 09:06:05.
corrected patch without changes in django/newforms/models.py
ticket-2076.4.diff (12.9 kB) - added by Ramiro Morales <rm0 _at_ gmx.net> on 02/27/07 06:10:41.
ticket-2076.5.diff (13.1 kB) - added by Ramiro Morales <rm0 _at_ gmx.net> on 02/27/07 13:25:52.
ticket-2076.6.diff (13.2 kB) - added by Ramiro Morales <rm0 _at_ gmx.net> on 02/28/07 12:26:24.
Same as ticket-2076.5.diff with corrections in the documentation and testcases comments
order.diff (0.8 kB) - added by Michael Radziej <mir@noris.de> on 03/19/07 10:54:17.

Change History

06/05/06 11:14:28 changed by oefe

Apparently, the select_related() workaround doesn't work for ForeignKey? with null=True.

06/20/06 19:20:48 changed by anonymous

  • cc set to gary.wilson@gmail.com.

06/21/06 16:58:53 changed by ramiro

See also #2210 where a fix is suggested

06/22/06 02:13:43 changed by Russell Cloran <russell@hbd.com>

The fix suggested will not fix this problem.

09/09/06 10:28:47 changed by serbaut@gmail.com

See patch in #2210.

11/16/06 19:11:25 changed by mrmachine <real dot human at mrmachine dot net>

  • cc changed from gary.wilson@gmail.com to gary.wilson@gmail.com, real.human@mrmachine.net.

01/10/07 04:28:11 changed by ramiro <rm0 _at_ gmx.net>

#3002 was a duplicate of this one.

01/24/07 12:37:27 changed by mir@noris.de

  • status changed from new to closed.
  • resolution set to duplicate.

duplicate of #2210, which has a working patch.

02/02/07 10:23:02 changed by ramiro <rm0 _at_ gmx.net>

  • attachment ticket-2076.diff added.

Patch originally posted by serbaut to #2210 with tests added

02/02/07 10:27:57 changed by ramiro <rm0 _at_ gmx.net>

Just attached a patch for this issue, created by serbaut and originally posted to ticket #2210 but IMHO this is better suited, see http://groups.google.ca/group/django-users/msg/c98e1631ce4b7c8e for the reasoning behind this.

The patch itself is updated to r4455 and I've added some tests of the new funcionality implemented by it.

(follow-up: ↓ 19 ) 02/02/07 11:43:05 changed by Michael Radziej <mir@noris.de>

  • keywords set to reopen.
  • has_patch set to 1.
  • stage changed from Unreviewed to Ready for checkin.

#2210 is actually a different issue, see this comment on #2210

Can somebody from core please reopen this bug?

I'm attaching a slightly modified patch, two backticks were missing in a docstring :-)

Thanks, serbaut for the original patch, and Ramiro Morales for the tests!

02/02/07 11:45:48 changed by Michael Radziej <mir@noris.de>

  • attachment ticket-2076.2.diff added.

slightly corrected version of previous patch (missing backticks)

02/02/07 14:45:18 changed by ramiro <rm0 _at_ gmx.net>

I will repeat/expand here some thing I've noted when implementing the tests so Django developers can take them in account when they decide if/when apply this patch:

  • The patch allows one to use 'relationship_name__related_model_field_name' ordering specification in the order_by() QuerySet method but not in the Meta.ordering attribute.
  • When you use a bare 'relationship_name' ordering specification the patch orders things by the related model PK and not by its Meta.ordering attribute. By examining the Admin app it seems that at least in that app the intent is to do the ordering by Meta.ordering if that exists and fallback to the PK otherwise. I don't know if this may be considered an inconsistency.
  • I have tested only ForeignKey relationships.
  • I have not tested if the ordering specification can be used by spanning through reverse relationships.
  • The patch does not seem to preserve backwards compatibility with the '[-]appname_tablename.fieldname' order_by specification notation suggested by the Django documentation (http://www.djangoproject.com/documentation/db_api/#order-by-fields). Would this be worth noting if the patch gets applied?.

02/06/07 10:30:27 changed by jacob

  • status changed from closed to reopened.
  • resolution deleted.

02/06/07 11:12:12 changed by Michael Radziej <mir@noris.de>

  • keywords deleted.

02/11/07 12:59:58 changed by Michael Radziej <mir@noris.de>

#3476 marked as duplicate.

(follow-up: ↓ 23 ) 02/19/07 18:51:23 changed by mtredinnick

  • status changed from reopened to new.
  • stage changed from Ready for checkin to Accepted.

This is really great work. Thanks to everybody's who's pitched in so far.

Not quite "ready for checkin" yet because of Ramiro's detective work when constructing the tests. A few things I would like to see working before this goes in:

  1. Should work with the !Meta inner class as well, since that provides default ordering for a model and it's going to be the source of a lot of complaints (from me, no less!) if you can only do something manually and not automatically.
  2. When just specifying a relation field as the ordering attribute, we should use the related model's "natural" ordering (that specified in Meta) before primary key ordering. Again, it's the intuitive behaviour when selecting a model on its own, so making it consistent across the board is worthwhile.
  3. Check it works with all relations (not just ForeignKeys).
  4. Let's work out what happens with backwards relations (and also spanning across multiple models: does model1__model2__some_field work?)

I'm not too worried about the backwards-compat issue, since that's a bit of an ugly hack anyway -- it's the only place you are ever required to know the database table names (or even care that the database exists) in the "non-direct-SQL" cases of a QuerySet. Providing we can get this fixed before 1.0, I'm willing to defend the case for breaking it in a small way.

The current patch is a great start and looks like the right way to approach the problem. So if somebody feels enthusiastic and wants to do some work on this, these are the things I'd like to see done before it can be checked in.

02/19/07 18:52:49 changed by mtredinnick

  • needs_better_patch set to 1.

02/20/07 04:34:31 changed by arthur.case@gmail.com

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com.

02/21/07 06:59:19 changed by Thomas Steinacher <tom@eggdrop.ch>

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch.

(in reply to: ↑ 10 ; follow-up: ↓ 20 ) 02/21/07 23:18:51 changed by ramiro <rm0 _at_ gmx.net>

Replying to Michael Radziej <mir@noris.de>:

... I'm attaching a slightly modified patch, two backticks were missing in a docstring :-)

it's me or an unrelated modification to django/newforms/models.py slipped in your ticket-2076.2.diff?.

(in reply to: ↑ 19 ) 02/22/07 09:03:31 changed by mir@noris.de

Replying to ramiro <rm0 _at_ gmx.net>:

it's me or an unrelated modification to django/newforms/models.py slipped in your ticket-2076.2.diff?.

You're right. It probably crept in from a different ticket when I forgot to clean up.

02/22/07 09:06:05 changed by mir@noris.de

  • attachment ticket-2076.3.diff added.

corrected patch without changes in django/newforms/models.py

02/22/07 15:59:31 changed by Michael Radziej <mir@noris.de>

Note: #2210 is an almost-duplicate and has been closed in favor of this ticket (it's a corner case). It should be checked separately that the solution for #2076 also fixes #2210.

02/24/07 13:12:08 changed by Lau Bech Lauritzen <lau__@ioa.dk>

Just stubbed my toe on this bug. It would probably be a good idea to remove the information in the documentation or change it to the workaround.

(in reply to: ↑ 15 ) 02/27/07 06:06:27 changed by Ramiro Morales <rm0 _at_ gmx.net>

Replying to mtredinnick:

Here is another iteration of the patch, not there yet because I don't have the required Django fu:

1. Should work with the !Meta inner class as well, since that provides default ordering for a model and it's going to be the source of a lot of complaints (from me, no less!) if you can only do something manually and not automatically.

This is working now, it wasn't a problem with the patch per se but a validation made in django/code/management.py:get_validation_errors() was triggering. I modified it to skip the validation just as it already does with the tablename.field form.

2. When just specifying a relation field as the ordering attribute, we should use the related model's "natural" ordering (that specified in Meta) before primary key ordering. Again, it's the intuitive behaviour when selecting a model on its own, so making it consistent across the board is worthwhile.

This is the one I haven't the skills to implement. serbaut@gmail.com implemented it by calling the lookup_inner function from QuerySet._get_sql_clause() but I don know what other internal function should be used to take in account a recursive scenario like this: The example at http://www.djangoproject.com/documentation/db_api/ with the Blog model having a relname relationship to a 4th model X and a Meta.ordering = (relname,) and trying to order the Entry model instances by its blog relationship field.

3. Check it works with all relations (not just ForeignKeys). 4. Let's work out what happens with backwards relations...

I've added (although a bit contrived) test cases for both ManyToMany and reverse ForeignKey and ManyToMany relationships . Please verify the coverage of these tests.

...(and also spanning across multiple models: does model1__model2__some_field work?)

There was already a test case for this.

I'm not too worried about the backwards-compat issue, since that's a bit of an ugly hack anyway -- it's the only place you are ever required to know the database table names (or even care that the database exists) in the "non-direct-SQL" cases of a QuerySet. Providing we can get this fixed before 1.0, I'm willing to defend the case for breaking it in a small way.

Well, there is something strange about this because as I see the original patch intended to maintain that backwards compatibility but it didn't achieve it, so it's a case of a broken attempt to maintain compatibility with a broken (as per your original report) but documented feature.

For now I've just eliminated a obvious dead code path but I didn't delete all the related code. I will attach another iteration of the patch where the tablename.filename is rejected in the validation and all traces of its support is eliminated from QuerySet._get_sql_clause().

Also, now db-api.txt is modified to describe the feature, please check and expand because my English is not very good (the same applies for the tests/modeltests/ordering/models.py mods).

Test and enhance so it can be included in 0.96 :)

02/27/07 06:10:41 changed by Ramiro Morales <rm0 _at_ gmx.net>

  • attachment ticket-2076.4.diff added.

02/27/07 13:24:10 changed by Ramiro Morales <rm0 _at_ gmx.net>

Replying to Ramiro Morales <rm0 _at_ gmx.net>:

... I will attach another iteration of the patch where the tablename.filename is rejected in the validation and all traces of its support is eliminated from QuerySet._get_sql_clause().

Done, see attached ticket-2076.5.diff

This is the one I haven't the skills to implement. serbaut@gmail.com implemented it by calling the lookup_inner function from QuerySet._get_sql_clause() but I don know what other internal function should be used to take in account a recursive scenario like this: The example at http://www.djangoproject.com/documentation/db_api/ with the Blog model having a relname relationship to a 4th model X and a Meta.ordering = (relname,) and trying to order the Entry model instances by its blog relationship field.

... or this better (worse?) scenario:

  • The example at http://www.djangoproject.com/documentation/db_api/
  • Blog model has a relname relationship to a 4th. model X
  • its has Meta.ordering = ('relname__a_field_from_model_X',)
  • We try to order the Entry model instances by its blog relationship field.

3. Check it works with all relations (not just ForeignKeys). 4. Let's work out what happens with backwards relations...

I've added (although a bit contrived) test cases for both ManyToMany and reverse ForeignKey and ManyToMany relationships . Please verify the coverage of these tests.

Almost forgot to add this note: When creating the doctests for ManyToManyField relationships I was getting duplicate objects in the order_by() query results so I needed to insert distinct() calls (see the last three tests), this may be sign of an issue/shortcomming with the patch when interacting with that kind of relationships. I can't compare this with any previous pre-patch behavior because the select_related() workaround doesn't work in the M2M case.

02/27/07 13:25:52 changed by Ramiro Morales <rm0 _at_ gmx.net>

  • attachment ticket-2076.5.diff added.

02/27/07 16:21:54 changed by anonymous

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch to mark@tranchant.co.uk.

02/27/07 16:22:13 changed by anonymous

  • cc changed from mark@tranchant.co.uk to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk.

02/28/07 12:26:24 changed by Ramiro Morales <rm0 _at_ gmx.net>

  • attachment ticket-2076.6.diff added.

Same as ticket-2076.5.diff with corrections in the documentation and testcases comments

03/16/07 05:24:05 changed by anonymous

There's one problem.

class A(models.Model):
  name = models.CharField()

class B(models.Model):
  [...]
  a = ForeignKey(A, null=True)

The following query will use an INNER JOIN to A:

B.objects.all().order_by(a__name)

This means that all objects B with B.a==None will be missing. So, this should use an OUTER JOIN. I'm not sure whether this qualifies the patch as wrong, since the ORM fails to switch to OUTER joins in similar circumstances with, too.

Michael

03/19/07 06:14:17 changed by Michael Radziej <mir@noris.de>

The supplied testcases do not work with a postgresql database, since it requires that there is an order-by clause that matches the distinct clause. I get these error messages:

======================================================================
FAIL: Doctest: modeltests.ordering.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mir/lib/python/django/test/doctest.py", line 2156, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for modeltests.ordering.models.__test__.API_TESTS
  File "/home/mir/src/django/trunk/tests/modeltests/ordering/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/mir/src/django/trunk/tests/modeltests/ordering/models.py", line ?, in modeltests.ordering.models.__test__.API_TESTS
Failed example:
    Reporter.objects.all().distinct()
Exception raised:
    Traceback (most recent call last):
      File "/home/mir/lib/python/django/test/doctest.py", line 1243, in __run
        compileflags, 1) in test.globs
      File "<doctest modeltests.ordering.models.__test__.API_TESTS[38]>", line 1, in ?
        Reporter.objects.all().distinct()
      File "/home/mir/lib/python/django/db/models/query.py", line 104, in __repr__
        return repr(self._get_data())
      File "/home/mir/lib/python/django/db/models/query.py", line 505, in _get_data
        self._result_cache = list(self.iterator())
      File "/home/mir/lib/python/django/db/models/query.py", line 189, in iterator
        cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
      File "/home/mir/lib/python/django/db/backends/postgresql/base.py", line 43, in execute
        return self.cursor.execute(sql, [smart_basestring(p, self.charset) for p in params])
    ProgrammingError: ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list

    SELECT DISTINCT "ordering_reporter"."id","ordering_reporter"."name" FROM "ordering_reporter" LEFT OUTER JOIN "ordering_reporter_artic
les" AS "m2m_ordering_reporter__articles" ON "ordering_reporter"."id" = "m2m_ordering_reporter__articles"."reporter_id" INNER JOIN "order
ing_article" AS "ordering_reporter__articles" ON "m2m_ordering_reporter__articles"."article_id" = "ordering_reporter__articles"."id" ORDE
R BY "ordering_reporter__articles"."headline" ASC

(and more for the other distinct queries). Unfortunately, the tests cannot be easily rephrased, since they really require distinct.

The postgresql docs state:

The DISTINCT ON expression(s) must match the leftmost ORDER BY expression(s).

Though, the patch works great for me ;-)

03/19/07 06:29:18 changed by Michael Radziej <mir@noris.de>

With postgresql, test 'reserved_names' also fails. There's a strange line in it:

>>> Thing.objects.order_by('select.when')

The model is:

class Thing(models.Model):
    when = models.CharField(maxlength=1, primary_key=True)
    join = models.CharField(maxlength=1)
    like = models.CharField(maxlength=1)
    drop = models.CharField(maxlength=1)
    alter = models.CharField(maxlength=1)
    having = models.CharField(maxlength=1)
    where = models.DateField(maxlength=1)
    has_hyphen = models.CharField(maxlength=1, db_column='has-hyphen')

    class Meta:
       db_table = 'select'

So, select.when does not make sense, but it used to work since the database table was called 'select'. IMHO, this is not a valid use. But the test case needs to be removed and the minor incompatibility should be noted.

03/19/07 10:53:56 changed by Michael Radziej <mir@noris.de>

Another test failure. The following test gives random results:

EditorResponse.objects.order_by('reader_letter__reader_name')

The appended patch fixes this test by requesting a second level ordering.

03/19/07 10:54:17 changed by Michael Radziej <mir@noris.de>

  • attachment order.diff added.

05/05/07 17:03:09 changed by anonymous

I'm a Django newbie and innocently stumbled into the recursive case that Ramiro mentioned:

class A(models.Model):
    ...

class B(models.Model):
    a = models.ForeignKey(A)
    ...
    class Meta:
        ordering = ('a',)

class C(models.Model):
    b = models.ForeignKey(B)
    ...
    class Meta:
        ordering = ('b',)

In svn trunk, trying to list Cs in the admin interface gave me a MySQL error that there was no field app_b.a

Because A just orders by the PK anyway, I was able to work around it by specifying 'a_id' in for B's ordering (though this raises an error in model validation that B has no 'a_id' field):

class B(models.Model):
    a = models.ForeignKey(A)
    ...
    class Meta:
        ordering = ('a_id',)

Just for kicks, I applied ticket-2076.6 patch, and now it won't work either way. The original code errors out looking for app_c.app_b.a and the hacked code looks for app_c.app_b.a_id

I know you're already aware that this is a problem with the patch. Just chiming in to note that this two-layer FK relationship is quite natural and easy to write, even for a beginner, so I would think that it needs to Just Work in 1.0.

05/29/07 13:20:04 changed by Wesley Fok <portland@chrominance.net>

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net.

06/04/07 22:16:37 changed by anonymous

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com.

06/15/07 04:31:51 changed by simonbun <simonbun@versea.be>

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be.

06/23/07 09:46:02 changed by anonymous

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz.

07/14/07 13:39:34 changed by anonymous

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz, eliott@cactuswax.net.

09/13/07 16:34:57 changed by mtredinnick

  • keywords set to qs-rf.

09/21/07 15:54:00 changed by Marek Kubica <pythonmailing@web.de>

  • cc changed from gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz, eliott@cactuswax.net to gary.wilson@gmail.com, real.human@mrmachine.net, arthur.case@gmail.com, tom@eggdrop.ch, mark@tranchant.co.uk, portland@chrominance.net, floguy@gmail.com, django@versea.be, remco@diji.biz, eliott@cactuswax.net, pythonmailing@web.de.

(follow-up: ↓ 40 ) 10/05/07 11:22:07 changed by miracle2k

Not sure if this has been mentioned already, but this patch breaks functionality to refer to columns added by extra().

(in reply to: ↑ 39 ) 10/05/07 11:31:18 changed by miracle2k

Replying to miracle2k:

Not sure if this has been mentioned already, but this patch breaks functionality to refer to columns added by extra().

To clarify: This is because the call to lookup_inner() in line 495 throws a TypeError? when it encounters the new column. I fixed this with a try-except-pass for me, but that probably means that I can't use both (extra-columns and related tables) at the same time.

10/08/07 16:26:28 changed by davep@atomicdroplet.com

FWIW I just tripped across what I think is this bug also. Briefly, I have a model a bit like this...

class DayLocationHit(models.Model):
    location=models.ForeignKey(Location)
    date=models.DateField()
    hits=models.IntegerField(default=0)
    
    class Admin:
        pass
    class Meta:
        ordering=('location','date') 

And can't use the foreign key as the "major" sort criteria in the admin interface. The exception is "missing FROM-clause entry for table "keywordsites_location" LINE 1: ...ques" FROM "keywordsites_daylocationhit" ORDER BY "keywordsi... " on Postgres.

10/14/07 17:38:55 changed by mtredinnick

(In [6510]) queryset-refactor: Fixed a large bag of order_by() problems.

This also picked up a small bug in some twisted select_related() handling.

Introduces a new syntax for cross-model ordering: foobarbaz, using field names, instead of a strange combination of table names and field names. This might turn out to be backwards compatible (although the old syntax leads to bugs and is not to be recommended).

Still to come: fixes for extra() handling, since the new syntax can't handle that and doc updates.

Things are starting to get a bit slow here, so we might eventually have to remove ordering by many-many and other multiple-result fields, since they don't make a lot of sense in any case. For now, it's legal.

Refs #2076, #2874, #3002 (although the admin bit doesn't work yet).

10/14/07 19:29:27 changed by mtredinnick

(In [6511]) queryset-refactor: Added an order_by parameter to extra(). Refs #2076.

10/14/07 19:30:29 changed by mtredinnick

  • keywords changed from qs-rf to qs-rf-fixed.

04/26/08 21:50:16 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

04/29/08 06:58:33 changed by aaron

This still seems broken to me (even after qs refactor, which is awesome in every other way, thanks Malcom!) when the related field may be null. For example:

# same as in modeltests.many_to_many_null.models
class Reporter(models.Model):
    name = models.CharField(max_length=30)

    def __unicode__(self):
        return self.name

class Article(models.Model):
    headline = models.CharField(max_length=100)
    reporter = models.ForeignKey(Reporter, null=True)

    class Meta:
        ordering = ('headline',)

    def __unicode__(self):
        return self.headline
"""
# Create a Reporter.
>>> r = Reporter(name='John Smith')
>>> r.save()

# Create an Article with no Reporter by passing "reporter=None".
>>> a = Article(headline="Ghost article", reporter=None)
>>> a.save()
>>> print a.reporter
None

>>> a = Article.objects.all()
>>> a
[<Article: Ghost article>]

>>> a = Article.objects.all().order_by('reporter__name')
>>> a
[<Article: Ghost article>]
"""

The last test there fails. But maybe I don't understand what the semantics should be here. It looks like the last test uses an inner join with reporter, when a left outer join might be more appropriate.

The same thing happens if you use 'reporter__name' in the default ordering in the Meta class of the Article model as well.


Add/Change #2076 (order_by with related table does not work)




Change Properties
Action