Re-open #7231: New "join" parameter for the "extra" QuerySet method

Showing 1-7 of 7 messages
Re-open #7231: New "join" parameter for the "extra" QuerySet method Ben Davis 3/8/11 1:35 PM
http://code.djangoproject.com/ticket/7231

I'd like to start a discussion on this since russelm closed the
issue.  There are a few other people that believe the issue should be
left open.   I've been using this patch for nearly two years, and have
found it to be useful in several different cases.  I disagree that
the  .raw() functionality is a sufficient alternative, as it is not
possible to modify an existing queryset using .raw().  For example,
if I have a function that accepts a queryset, I want to be able to
modify that queryset by giving it a extra info for the JOIN and SELECT
clauses.
Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Jacob Kaplan-Moss 3/8/11 3:09 PM

.extra() was a kludge that existed because .raw() didn't. Frankly, I'm
considering deprecating and removing .extra() entirely: I've rarely
seen a case where using it didn't come back to cause problems in the
future. I'm certainly going to be a strong -1 on adding any more
"features" to .extra().

Remember, Django's ORM tries to hit an 80-90% point, but then you're
*expected* to fall back to raw SQL past that point. Falling back to
raw SQL when it's easier is a feature, not a bug.

Jacob

Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Russell Keith-Magee 3/8/11 3:16 PM
On Wed, Mar 9, 2011 at 7:09 AM, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> On Tue, Mar 8, 2011 at 9:35 PM, bendavis78 <benda...@gmail.com> wrote:
>> I'd like to start a discussion on this since russelm closed the
>> issue.  There are a few other people that believe the issue should be
>> left open.   I've been using this patch for nearly two years, and have
>> found it to be useful in several different cases.  I disagree that
>> the  .raw() functionality is a sufficient alternative, as it is not
>> possible to modify an existing queryset using .raw().  For example,
>> if I have a function that accepts a queryset, I want to be able to
>> modify that queryset by giving it a extra info for the JOIN and SELECT
>> clauses.
>
> .extra() was a kludge that existed because .raw() didn't. Frankly, I'm
> considering deprecating and removing .extra() entirely:

Yes. Yes Yes Yes. Yes. Oh, and Yes. +1.

> I've rarely
> seen a case where using it didn't come back to cause problems in the
> future. I'm certainly going to be a strong -1 on adding any more
> "features" to .extra().

Agreed. From an engineering perspective, extra() is the single most
fragile part of the ORM. Dumping extra would make me extraordinarily
happy.

Yours,
Russ Magee %-)

Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Dan Watson 3/9/11 2:06 PM
It's also incredibly useful for certain situations. Two that come immediately to mind are augmenting objects returned with an extra field via custom SQL (a subselect for instance), and using custom SQL (e.g. to_tsquery for full-text searches in Postgres) in the WHERE clause. Both *could* be accomplished using .raw() if you don't mind giving up .filter(), .order_by(), etc. But it would be a shame to throw out everything a queryset gives you just because you need hooks into the SQL it generates.

If you're seriously considering deprecating .extra(), I think it would be beneficial to put together some use cases (such as those above) that could be covered in a less-fragile way.

Dan

Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Ben Davis 3/12/11 9:06 AM

Even if it is a kludge, it still accomplishes something that .raw() cannot (as Dan put forth).  I think deprecating it in favor of raw doesn't make much sense, since they are two different things.

> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Matthieu Rigal 3/10/14 10:37 AM
Sorry to reopen an old thread, but this was is indicated to do in the ticket doc.

I think actually extra is a very helpful function and that join is a missing part of it. I'm thinking especially about the admin part, where one sometime wants to add count of foreign keys to the view. Since the aggregate is so unperformant and returns bad results if you want more than one count, but you also want to keep the filtering and ordering features of the admin interface, there is no better solution than join as extra.

One example : LEFT OUTER JOIN (SELECT COUNT(*) AS "devices_count", "accounts_venuedevice"."venue_id" FROM "accounts_venuedevice" WHERE "accounts_venuedevice"."enabled"=true GROUP BY "accounts_venuedevice"."venue_id" ) AS "enabled_devices" ON ( "accounts_venue"."id" = "enabled_devices"."venue_id") 

raw() is not a good option for this case and the classic ORM neither is an option because of wrong and slow queries...

Best,
Matthieu
Re: Re-open #7231: New "join" parameter for the "extra" QuerySet method Josh Smeaton 3/10/14 5:57 PM
My take on this is that .extra and .raw are 'hacks' that only exist because the existing functions of the ORM are too limited. There are a few changes coming to extend the utility of .annotate (and .aggregate), along with the Lookup and Transform APIs, that should solve 90% of use cases that .extra is currently used for. Therefore I don't think that extending the utility of .extra is the right way forward.

Even with the coming changes mentioned above, there are still limitations, and forcing parts of a query into a subquery is not really supported. Trying to use a negated Q() with m2m joins is an example I believe, and so is aggregating over m2m joins (someone will correct me if I'm wrong - I'm sure).

I think time and effort would be better spent on designing a solution to the above issues. Being able to force a particular query into a subquery will be extremely useful, and will solve the bugs causing you to ask for a .join implementation. I totally agree that there is a need for join handling, but I think it's better left as an internal django function that does the correct thing when needed.

Regards,

Josh