#2076: order_by with related table does not work
---------------------------+------------------------------------------------
Reporter: mtredinnick | Owner: mtredinnick
Status: new | Component: Database wrapper
Version: SVN | Resolution:
Keywords: | Stage: Accepted
Has_patch: 1 | Needs_docs: 0
Needs_tests: 0 | Needs_better_patch: 1
---------------------------+------------------------------------------------
Comment (by Ramiro Morales <rm0 _at_ gmx.net>):
Replying to [comment:15 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. [EMAIL PROTECTED]
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 :)
--
Ticket URL: <http://code.djangoproject.com/ticket/2076#comment:23>
Django Code <http://code.djangoproject.com/>
The web framework for perfectionists with deadlines
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
