Opened 8 years ago

Closed 7 years ago

Last modified 3 years ago

#27149 closed New feature (fixed)

Allow using a subquery in QuerySet.filter()

Reported by: MikiSoft Owned by: Matthew Schinckel
Component: Database layer (models, ORM) Version:
Severity: Normal Keywords: Queryset SubQuery Exists
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by MikiSoft)

The following function is used for filtering by generic relation (and also by one column in the model where it is) which isn't natively supported by Django.

APP_LABEL = os.path.basename(os.path.dirname(__file__))
def generic_rel_filter(model, target, column, id):
    return model.objects.extra(where=['''
        {app_label}_{model}.id in (select object_id
        from {app_label}_{target}
        where content_type_id = (select id from django_content_type where model = '{model}')
            and {column} = {id})'''.format(app_label=APP_LABEL, model=model.__name__.lower(), target=target, column=column, id=id)])

Example: If I have Event and Like model, and the second one has generic relation to the first one (i.e. it has content_type, object_id and content_object fields), then if I want to get all events which current user liked, I would just make this call in a view: generic_rel_filter(Event, 'like', 'person', self.request.user.pk)

Note that this function isn't intended to be used with user specified parameters, otherwise it's prone to SQL injection attacks.

P.S. It can be done with ORM but then it would go with three queries, which is much slower than the method above (which uses only one query to do the same): Event.objects.filter(pk__in=Like.objects.filter(content_type=ContentType.objects.get(model='event'), person=self.request.user).values_list('object_id', flat=True))

Change History (16)

comment:1 by MikiSoft, 8 years ago

Description: modified (diff)

comment:2 by MikiSoft, 8 years ago

Description: modified (diff)

comment:3 by Shai Berger, 8 years ago

Hi,

Thanks for the suggestion; the code, as given, is not fit for general use because it makes assumptions which are not guaranteed to hold:

  • it assumes the model and target belong to the same app
  • it assumes the modles use the default table names (i.e. none of the models specifies Meta.db_table)
  • it assumes the default app label (a different one can be set via AppConfig objects)
  • it assumes the names content_type_id and object_id for the components of the generic FK -- these are not even defaults, but only a convention

Further, the code uses target in a way which bakes some of these assumptions into the API, not just the implementation.

I suggest you bring the idea up on the DevelopersMailingList, to get a wider discussion of whether the feature fits for inclusion in Django, and if so, to define an API everyone agrees on. When you do, please make sure you present the problem before you suggest your solution.

comment:4 by Tim Graham, 8 years ago

I think the intention of this ticket was to present a case where QuerySet.extra is required. If we want to deprecate it, we need to provide an alternative for accomplishing this.

comment:5 by MikiSoft, 8 years ago

Exactly, timgraham. Thanks for clearing it up instead of me. :)

By the way, I've begun to use QuerySet.extra whenever there are some nested queries (i.e. more than one), and I think that execution is now noticeably faster than earlier.

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

comment:6 by Tim Graham, 8 years ago

Summary: Filtering with generic relationAllow using a subquery in QuerySet.filter()
Triage Stage: UnreviewedAccepted

A PR provides the ability to annotate with SubQuery and Exists. I think this is a bit different since it applies to QuerySet.filter() rather than annotate().

comment:7 by Matthew Schinckel, 7 years ago

Has patch: set
Keywords: Queryset SubQuery Exists added; QuerySet.extra removed
Owner: changed from nobody to Matthew Schinckel
Patch needs improvement: set
Status: newassigned

For what it's worth, I have a PR about this (thanks to whoever pointed this issue out).

However, there is still one outstanding issue related to using .filter(fooin=SubQuery(...))

Feel free to jump in with suggestions about how to resolve it at https://github.com/django/django/pull/6478

Version 0, edited 7 years ago by Matthew Schinckel (next)

comment:8 by Tim Graham, 7 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 236ebe94:

Fixed #27149 -- Added Subquery and Exists database expressions.

Thanks Josh Smeaton for Oracle fixes.

comment:10 by Tim Graham <timograham@…>, 5 years ago

In 96b6ad9:

Refs #27149 -- Made Subquery store Query instead of Queryset.

Subquery only uses Query.

comment:11 by Tim Graham <timograham@…>, 5 years ago

In 35431298:

Refs #27149 -- Moved subquery expression resolving to Query.

This makes Subquery a thin wrapper over Query and makes sure it respects
the Expression source expression API by accepting the same number of
expressions as it returns. Refs #30188.

It also makes OuterRef usable in Query without Subquery wrapping. This
should allow Query's internals to more easily perform subquery push downs
during split_exclude(). Refs #21703.

comment:12 by Tim Graham <timograham@…>, 5 years ago

In 3a505c7:

Refs #27149, #29542 -- Simplified subquery parentheses wrapping logic.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In c0969ee:

Refs #27149 -- Based recursive nested subquery detection on sys.getrecursionlimit().

This makes sure the test_avoid_infinite_loop_on_too_many_subqueries test
doesn't fail on systems with a non-default recursion limit.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In bbf141b:

Refs #27149 -- Fixed sql.Query identity.

By making Query subclass BaseExpression in
35431298226165986ad07e91f9d3aca721ff38ec the former defined it's
identity based off _construct_args which is not appropriate.

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 6d0cbe42:

Fixed #32650 -- Fixed handling subquery aliasing on queryset combination.

This issue started manifesting itself when nesting a combined subquery
relying on exclude() since 8593e162c9cb63a6c0b06daf045bc1c21eb4d7c1 but
sql.Query.combine never properly handled subqueries outer refs in the
first place, see QuerySetBitwiseOperationTests.test_subquery_aliases()
(refs #27149).

Thanks Raffaele Salmaso for the report.

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 48e19bae:

[3.2.x] Fixed #32650 -- Fixed handling subquery aliasing on queryset combination.

This issue started manifesting itself when nesting a combined subquery
relying on exclude() since 8593e162c9cb63a6c0b06daf045bc1c21eb4d7c1 but
sql.Query.combine never properly handled subqueries outer refs in the
first place, see QuerySetBitwiseOperationTests.test_subquery_aliases()
(refs #27149).

Thanks Raffaele Salmaso for the report.

Backport of 6d0cbe42c3d382e5393d4af48185c546bb0ada1f from main

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