Opened 7 years ago

Closed 6 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 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 by MikiSoft, 7 years ago

Description: modified (diff)

comment:2 by MikiSoft, 7 years ago

Description: modified (diff)

comment:3 by MikiSoft, 7 years ago

Type: UncategorizedNew feature

comment:4 by MikiSoft, 7 years ago

Description: modified (diff)

comment:5 by Tim Graham, 7 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #26426.

in reply to:  5 comment:6 by MikiSoft, 7 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.

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

comment:7 by MikiSoft, 7 years ago

Resolution: duplicate
Status: closednew

comment:8 by MikiSoft, 7 years ago

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

comment:9 by Tim Graham, 7 years ago

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 by Nicolas Delaby, 7 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:11 by MikiSoft, 7 years ago

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

comment:12 by Anssi Kääriäinen, 7 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=...)
Last edited 7 years ago by Tim Graham (previous) (diff)

comment:13 by Simon Charette, 7 years ago

Cc: Simon Charette added

comment:14 by Nicolas Delaby, 7 years ago

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

comment:15 by Daniel Hahler, 7 years ago

Cc: django@… added

comment:17 by Nicolas Delaby, 7 years ago

Cc: ticosax@… added

comment:18 by Claude Paroz, 7 years ago

Has patch: set

comment:19 by Tim Graham, 7 years ago

Patch needs improvement: set

comment:20 by Daniel Hahler, 7 years ago

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

comment:21 by Josh Smeaton, 7 years ago

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 by Tim Graham, 7 years ago

Patch needs improvement: unset
Version: master

comment:23 by Tim Graham <timograham@…>, 6 years ago

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