On Fri, Sep 4, 2009 at 6:30 AM, Ben Davis<bendavi...@gmail.com> wrote:
> Hi I've submitted a patch for this bug that I'd like to have considered for
> some trunk action... I can't take the credit for all the work on this, but
> it seems I'm taking the initiative to get it resolved.
... which is exactly what is needed. Having patches is great, but what
we really need is people willing to engage the core and the
development community over the long run to see fixes like this one
home. Persistence will be required :-)
> I really only had one question about a particular change that was needed in
> order for the .values() query to work with reverse one-to-one (e.g.,
> User.objects.values('username', 'userprofile__zip') ) . It looks like
> mtredinnick wrote the original code for this that was modified. The change
> was fairly simpe, In the setup_joins function in db/models/sql/query.py:
>
> ---
>
> raise FieldError("Cannot resolve keyword %r into field.
> "
> "Choices are: %s" % (name, ", ".join(names)))
>
>
> - if not allow_many and (m2m or not direct):
> + if not allow_many and m2m:
> for alias in joins:
> self.unref_alias(alias)
> raise MultiJoin(pos + 1)
>
>
> ---
>
> Looking at the original code, I'm not sure why "indirect" fields were not
> allowed if they weren't many-to-many relationships. This is basically what
> was keeping the reverse one-to-one lookups from working. I'm not 100% sure
> of the consequences of this change, but all model tests seemed to pass with
> this change.
>
> thoughts?
My immediate thought is that the effect of the (m2m or not direct)
logic is to remove the 'many' sides of fields from consideration. The
comparison you're looking at is operating on data that comes from line
1725:
field, model, direct, m2m = opts.get_field_by_name(name)
In this contect, 'field' can be one of 5 things:
1. Natural fields (charfield)
2. ForeignKey fields (the local end)
3. OneToOneFields (the local end)
4. ManyToManyField (the local end)
5. RelatedObject (the remote 'many' end of a FK, or the remote end of an m2m)
6. RelatedObject (the remote end of a OneToOne Field)
(1) is not of interest here. select_related() currently works with (2)
and (3). These are the values where direct=True (since they are
locally defined) but m2m is False. (4) always returns m2m; (5) and (6)
always return direct=False.
(6) is the case you're trying allow, but you can't do so at the
expense of excluding (5).
The fact that the tests don't fail when you make this change is
promising. (I presume your statement about model tests includes the
regression tests too, not just the modeltest directory) However,
there's no particular guarantee of branch coverage in Django's test
suite.
Just be sure, my suggestion here it to try and be pathological. Work
your way backwards from that if statement, and try to work out what
set of models will cause that join condition to be invoked.
Essentially, try to reverse engineer the test case that will fail if
you modify that line. If it turns out to not be possible, then explain
why; if it is possible, then you need to either find another way to
manufacture the join you need.
Yours,
Russ Magee %-)
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
