Opened 8 years ago
Closed 7 years ago
#27332 closed New feature (fixed)
Specifying additional ON arguments, and more flexibility with joins
Reported by: | MikiSoft | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette, django@…, ticosax@…, josh.smeaton@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
For example, this raw SQL:
SELECT event.* FROM event INNER JOIN business ON (event.business_id = business.id AND business.manager_id = 1) LEFT JOIN 'like' ON (event.id = object_id AND content_type_pk = 17 AND person_id = 1) ORDER BY COALESCE('like'.date, event.'when') DESC;
Can't be translated to the equivalent Django ORM expression even if using extra()
command, because there isn't anywhere a possibility to set additional arguments on JOIN
clauses. And when I'm using filter(Q(argument) | ...)
it happens to always push all arguments to WHERE
clause, which just causes a performance hit. I wish there was some parameter which would determine the destination of the argument, whether it has to go to the (last) JOIN
clause, or to be put in WHERE
(which goes by default). Also, I'm not able to comprehend how to perform correctly LEFT JOIN
so in the end I have made some jumbled up expression which is completely inefficient (as it executes two queries, but it's the only way to do the same from above and get QuerySet
in return):
Event.objects.filter(Q(pk__in=Like.objects.filter(person_id=1, content_type_id=17).prefetch_related('content_object').values_list('object_id', flat=True)) | Q(business__manager_id=1)).order_by(Coalesce('likes__date', 'when').desc())
I'm filing this issue because some (or probably many) people like me desperately need output in a form of a QuerySet
because of the pagination and many other things RawQuerySet
doesn't support, so using raw()
isn't definitely an option in my case. Django has left me out of choice.
Change History (23)
comment:1 by , 8 years ago
Description: | modified (diff) |
---|
comment:2 by , 8 years ago
Description: | modified (diff) |
---|
comment:3 by , 8 years ago
Type: | Uncategorized → New feature |
---|
comment:4 by , 8 years ago
Description: | modified (diff) |
---|
follow-up: 6 comment:5 by , 8 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:6 by , 8 years ago
Tim Graham,
I understand that it had a duplicate title with the issue you've mentioned, but only in that terms, because the problems differ - in that issue the equivalent solution is provided using extra()
and aggregate()
method, while here, there isn't any possible one. It addresses the different scope of the problem described in that title, which technically (if pointed out on efficiency like always), is unsolvable using current ORM expressions. Therefore, I expect from you or other people involved in Django development to provide or guide me towards the proper solution (using any of the ORM expressions, including extra()
of course), as I certainly can't help myself, because I really tried hard focusing on and examining ORM features in detail, and asking others - but no avail. I doubt that anyone would solve it in an acceptable way until some new feature is added which would allow such thing to be achieved, so I think that this definitely deserves to be a separate issue.
comment:7 by , 8 years ago
Resolution: | duplicate |
---|---|
Status: | closed → new |
comment:8 by , 8 years ago
Summary: | Specifying additional JOIN arguments → Specifying additional ON arguments, and more flexibility with joins |
---|
comment:9 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I don't know. I'll tentatively accept the ticket but it might not be feasible to construct every query through the ORM.
comment:10 by , 8 years ago
Hi,
I started to look at this one and realized GenericForeignKey
implemented such functionnality in get_extra_restriction()
https://github.com/django/django/blob/master/django/contrib/contenttypes/fields.py#L437
So I followed that path, and came up with a PoC implementation (with passing tests), following this API:
class ConditionalJoinTests(TestCase): @classmethod def setUpTestData(cls): cls.author1 = Author.objects.create(name='Alice') cls.author2 = Author.objects.create(name='Jane') cls.book1 = Book.objects.create(title='Poem by Alice', editor='A', author=cls.author1) cls.book2 = Book.objects.create(title='The book by Jane A', editor='A', author=cls.author2) cls.book2 = Book.objects.create(title='The book by Jane B', editor='B', author=cls.author2) def test_conditional_join_query_wo_join(self): """ All Authors are returned because no join is required by the filters. """ self.assertQuerysetEqual( Author.objects .conditional_join('book', title__iexact='poem by alice'), ["<Author: Alice>", "<Author: Jane>"]) def test_conditional_join_query_with_join(self): self.assertQuerysetEqual( Author.objects .conditional_join('book', title__iexact='poem by alice') .filter(book__isnull=False), ["<Author: Alice>"]) self.assertQuerysetEqual( Author.objects .conditional_join('book', Q(title__iexact='poem by alice')) .filter(book__isnull=False), ["<Author: Alice>"]) def test_conditional_join_query_with_join_multiple(self): self.assertQuerysetEqual( Author.objects .conditional_join('book', title__icontains='jane', editor='B') .filter(book__isnull=False), ["<Author: Jane>"])
If this proposal receives some interest, I'll be glad to work on preparing a pull request to support this use case.
comment:12 by , 8 years ago
This is something I've wanted for a long time.
Some considerations:
- In general the ORM API tries to avoid SQL specific terms like join. Maybe relation would be a better term to use?
- The API shouldn't edit the current relation, instead it should add a new lookup path alias. So:
.filtered_relation('translations', alias='translation_fi', condition=Q(translations__lang='fi')).filter(translation_fi__title=...)
comment:13 by , 8 years ago
Cc: | added |
---|
comment:15 by , 8 years ago
Cc: | added |
---|
comment:17 by , 8 years ago
Cc: | added |
---|
comment:18 by , 8 years ago
Has patch: | set |
---|
comment:19 by , 8 years ago
Patch needs improvement: | set |
---|
comment:20 by , 8 years ago
PR review comments have been addressed.
Please consider including it for Django 1.11.
comment:21 by , 8 years ago
Cc: | added |
---|
Sorry, I wasn't aware this patch existed, or I would have tried to review for 1.11. I'll make some time to review this patch in the next few weeks.
comment:22 by , 7 years ago
Patch needs improvement: | unset |
---|---|
Version: | → master |
Duplicate of #26426.