Skip to content

Fixed #37115, Refs #5929 -- Added a CompositeField expression to support multi-column subqueries. - #21636

Open
p-r-a-v-i-n wants to merge 97 commits into
django:mainfrom
p-r-a-v-i-n:clean-implementation-of-composite-field
Open

p-r-a-v-i-n wants to merge 97 commits into
django:mainfrom
p-r-a-v-i-n:clean-implementation-of-composite-field

Conversation

@p-r-a-v-i-n

@p-r-a-v-i-n p-r-a-v-i-n commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Trac ticket number

ticket-37115
ticket-5929

Branch description

This PR Introduces a CompositeField to support subqueries which return multi-cols.

Currently Django ORM does not have internal machinery to decide the output_field of subqueries which have multi-columns in their output.

CompositeField will resolve output_field for such subqueries.

Scope of this PR:

  1. Scope of CompositeField for this PR is limited to virtual sources only.
  2. selecting multi-column subquery result as annotation not support.
  3. Correlated(using OuterRef) subquery not supported .

References:

AI Assistance Disclosure (REQUIRED)

  • No AI tools were used in preparing this PR.
  • 1. I have used Chatgpt 5.5, for creating intial data for testing, and I have used it for researching internal of Orm.
    2. I also took help to write documentation part.

Checklist

  • This PR follows the contribution guidelines.
  • This PR does not disclose a security vulnerability (see vulnerability reporting).
  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number (if applicable), and ends with a period (see guidelines).
  • I have not requested, and will not request, an automated AI review for this PR.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Comment thread django/db/models/sql/compiler.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment on lines +1705 to +1713
clause = WhereNode([condition], connector=AND)
if (
current_negated
and condition.lookup_name != "isnull"
and condition.rhs is not None
and self._expression_contains_nullable_subquery_column(condition.lhs)
):
lookup_class = condition.lhs.get_lookup("isnull")
clause.add(lookup_class(condition.lhs, False), AND)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is for the case when we have something
inner --> which don't have email = 'bob@mail.com'. but in the outer query if we are doing
outer --> .exclude(info__email='bob@mail.com') in this case
we are appending here is_null=False condition in the where node. so we should get explicit true or false. not unknown.

Before:

WHERE NOT ( 'info'.email == 'bob@mail.com'') but info.email is NULL, so it becaming 
WHERE NOT ( null == 'bob@mail.com' )  --> 
WHERE NOT  UNKNOWN  will remove the outer row.

After:

NULL == "'bob@mail.com" --> unknown
AND
NULL IS NOT NULL --> FALSE

UNKOWN FALSE --> FALSE

NOT FALSE --> TRUE --> so the outer row is preserved.

I may sound unclear here , please don't hesitate to ask again.

@p-r-a-v-i-n
p-r-a-v-i-n marked this pull request as ready for review July 17, 2026 09:23
@p-r-a-v-i-n

p-r-a-v-i-n commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor Author

#21496 (comment) and Jacob's suggestion

As a first step, let's avoid that. Is there a way to add the JOIN "just-in-time" when a filter needs it?

helped me to come up with this idea.

Just high-level overview of the current approach ( may be I should have put this into the PR body ):

We are not changing the existing behavior for single-column subqueries. We trigger this new logic only when these .filter(), .values(), or .order_by(). methods reference the multi-column alias
For example, when we receive:
.filter(info__email="bob@mail.com")
we split the name and check whether info is present in self.annotations. If it is present, we check whether the annotation is a multi-column subquery.

If it is a multi-column subquery, we check whether a SubqueryJoin for this annotation is already present in alias_map. If it is present, we reuse it. Otherwise, we create a new SubqueryJoin and register it in alias_map.

The SubqueryJoin puts the inner query into the outer query’s FROM clause:

LEFT OUTER JOIN (
    SELECT email, name
    FROM ...
) info ON (1 = 1)

After that:
info__email
is resolved into an individual Col pointing to info.email.
If the whole annotation is used, for example in a tuple lookup, we resolve it into an internal Tuple containing all the columns.

this way, the inner multi-column query is added to the FROM clause only when one of its fields, or the complete tuple, is actually needed. The derived table appears only once in the generated SQL, and multiple references reuse the same SubqueryJoin

@charettes
charettes self-requested a review July 17, 2026 12:35
@jacobtylerwalls jacobtylerwalls added geodjango coverage Generate a comment with a coverage diff labels Jul 19, 2026
@p-r-a-v-i-n

This comment has been minimized.

@jacobtylerwalls

This comment has been minimized.

@p-r-a-v-i-n

This comment has been minimized.

@jacobtylerwalls jacobtylerwalls left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simpler version looks promising!

Comment thread django/db/models/lookups.py Outdated
Comment thread django/db/models/sql/compiler.py Outdated
Comment thread django/db/models/sql/datastructures.py Outdated
Comment thread django/db/models/fields/composite.py Outdated
class CompositeField(Field):
is_composite = True

def __init__(self, **kwargs):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As on the previous PR, I think we should add support for *args without names. I suppose we could do this in another PR that adds test cases for Func() subclasses instead of Subquery, as I imagine that to be the use case for anonymous args.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #21636 (comment) for more discussion about *args.

Comment thread django/db/models/sql/compiler.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py
Comment thread django/db/models/sql/query.py
Comment thread django/db/models/sql/query.py Outdated
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the clean-implementation-of-composite-field branch 2 times, most recently from 19d7423 to 05fa1eb Compare July 21, 2026 15:13
@jacobtylerwalls

Copy link
Copy Markdown
Member

buildbot, test on oracle.

@jacobtylerwalls jacobtylerwalls left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a manual test against a PR for djangoproject.com, and I have a couple of observations.

Comment thread tests/composite_field/test_composite_field.py
Comment thread tests/composite_field/test_composite_field.py Outdated
Comment thread tests/composite_field/test_composite_field.py Outdated
Comment thread django/db/models/fields/composite.py Outdated
Comment thread django/db/models/sql/query.py
Comment thread docs/ref/models/expressions.txt Outdated
Comment thread docs/releases/6.2.txt Outdated
Comment thread django/db/models/fields/composite.py Outdated
Comment thread docs/ref/models/expressions.txt Outdated
Comment thread docs/releases/6.2.txt Outdated
Comment thread docs/releases/6.2.txt

These subfields can be selected, filtered, annotated, and ordered in the same
way as model fields. See :ref:`using-multi-column-subqueries` for more details.

@jacobtylerwalls jacobtylerwalls Aug 7, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a follow-up PR, we could walk through the example from jacobtylerwalls/djangoproject.com@219e530 in the contrib.postgres docs for SearchHeadline, and show an example like this:

Since SearchHeadline can be expensive to calculate, the query below avoids
annotating it on all results before the limit is taken, and also avoids
re-expressing the rank::

subquery = (
    Product.objects
    .annotate(rank=SearchRank(...))
    .values("pk", "rank")
    .order_by("-rank", "pk")
)
query = (
    Product.objects
    .alias(ranked=subquery[:10])
    .filter(pk__in=F("ranked__pk")
    .annotate(headline=SearchHeadline(...))
    .order_by("-ranked__rank", "pk")
)

or similar.

If we do that, then we could possibly link to that here, idk.

(But we can leave for a follow-up, to give others a chance to weigh in.)

@p-r-a-v-i-n p-r-a-v-i-n Aug 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked it. That will add more merits (in my POV).

@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the clean-implementation-of-composite-field branch from 637cbe3 to 2acd3cd Compare August 9, 2026 14:33

@jacobtylerwalls jacobtylerwalls left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really appreciate the incremental commits including test cases with each change, nice work on that!

I did another holistic review, and then my plan is to look a little closer at the join machinery parts to see if we can squeeze out any simplifications.

Comment thread django/db/models/lookups.py Outdated
)


class CompositeFieldOutputFieldTests(SimpleTestCase):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might burst this file into modules. You don't have to use as many as tests/composite_pk did, but it's probably good to have more than just one module.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can hold this comment until the end, since I just praised all your atomic commits with tests 🤣

Comment thread django/db/models/expressions.py Outdated
Comment thread django/db/models/fields/tuple_lookups.py
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread tests/composite_field/test_composite_field.py Outdated
Comment thread django/db/models/sql/query.py
Comment thread django/db/models/sql/query.py Outdated
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the clean-implementation-of-composite-field branch from ed7559a to b7c33e5 Compare August 13, 2026 08:25
Comment thread tests/composite_field/test_composite_field.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/lookups.py Outdated
Comment thread django/db/models/sql/query.py Outdated
Comment thread django/db/models/sql/query.py Outdated
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the clean-implementation-of-composite-field branch from 349539b to 11869c9 Compare October 14, 2026 16:05
@github-actions

Copy link
Copy Markdown

Uncovered lines in changed files

-------------
django/db/models/__init__.py (100%)
django/db/models/expressions.py (100%)
django/db/models/fields/__init__.py (100%)
django/db/models/fields/composite.py (100%)
django/db/models/fields/reverse_related.py (100%)
django/db/models/fields/tuple_lookups.py (100%)
django/db/models/lookups.py (100%)
django/db/models/sql/compiler.py (100%)
django/db/models/sql/datastructures.py (98.0%): Missing lines 253
django/db/models/sql/query.py (98.8%): Missing lines 387,398
-------------
Total:   313 lines
Missing: 3 lines
Coverage: 99%
-------------

Note: Missing lines are warnings only. Some database-specific lines may not be measured. More information

@jacobtylerwalls jacobtylerwalls changed the title Refs #5929 -- Added a CompositeField expression to support multi-column subqueries. Fixed #37115, Refs #5929 -- Added a CompositeField expression to support multi-column subqueries. Oct 15, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Apply to have ASV benchmarks run on a PR coverage Generate a comment with a coverage diff geodjango

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants