Opened 2 years ago

Closed 15 months 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: master
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 MikiSoft)

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 Changed 2 years ago by MikiSoft

Description: modified (diff)

comment:2 Changed 2 years ago by MikiSoft

Description: modified (diff)

comment:3 Changed 2 years ago by MikiSoft

Type: UncategorizedNew feature

comment:4 Changed 2 years ago by MikiSoft

Description: modified (diff)

comment:5 Changed 2 years ago by Tim Graham

Resolution: duplicate
Status: newclosed

Duplicate of #26426.

comment:6 in reply to:  5 Changed 2 years ago by MikiSoft

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.

Last edited 2 years ago by MikiSoft (previous) (diff)

comment:7 Changed 2 years ago by MikiSoft

Resolution: duplicate
Status: closednew

comment:8 Changed 2 years ago by MikiSoft

Summary: Specifying additional JOIN argumentsSpecifying additional ON arguments, and more flexibility with joins

comment:9 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

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 Changed 2 years ago by Nicolas Delaby

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:11 Changed 2 years ago by MikiSoft

Awesome! Looking forward to its implementation in Django. :)

comment:12 Changed 2 years ago by Anssi Kääriäinen

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=...)
Last edited 2 years ago by Tim Graham (previous) (diff)

comment:13 Changed 2 years ago by Simon Charette

Cc: Simon Charette added

comment:14 Changed 2 years ago by Nicolas Delaby

Thanks Anssi for your feedback. I'll start from your proposal.

comment:15 Changed 2 years ago by Daniel Hahler

Cc: django@… added

comment:17 Changed 2 years ago by Nicolas Delaby

Cc: ticosax@… added

comment:18 Changed 2 years ago by Claude Paroz

Has patch: set

comment:19 Changed 2 years ago by Tim Graham

Patch needs improvement: set

comment:20 Changed 2 years ago by Daniel Hahler

PR review comments have been addressed.
Please consider including it for Django 1.11.

comment:21 Changed 22 months ago by Josh Smeaton

Cc: josh.smeaton@… 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 Changed 15 months ago by Tim Graham

Patch needs improvement: unset
Version: master

comment:23 Changed 15 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 01d440fa:

Fixed #27332 -- Added FilteredRelation API for conditional join (ON clause) support.

Thanks Anssi Kääriäinen for contributing to the patch.

Note: See TracTickets for help on using tickets.
Back to Top