Fixed #36464 -- Fixed tuple in subquery lookup when missing native support. #19565

charettes · 2025-06-14T15:10:21Z

Trac ticket number

Branch description

When native support is missing it can be emulated with an EXISTS clause.

The mishandling of subquery right-hand-side was likely missed because the only core backend we happen to test with the supports_tuple_lookups feature disabled (Oracle < 23.4) supports it natively.

Thanks @NandanaRaol for the report against SQL Server.

charettes · 2025-06-14T15:18:41Z

buildbot, test on oracle.

sarahboyce · 2025-06-16T10:06:48Z

Thank you for picking this up Simon ⭐

I have been trying and failing to run the mssql-django tests following the contributing instructions using docker to run the tests

Error in case this is obvious

    conn = Database.connect(connstr, **args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.Error: ('01000', "[01000] [unixODBC][Driver Manager]Can't open lib 'FreeTDS' : file not found (0) (SQLDriverConnect)")

This means I'm struggling to confirm the issue is fixed.
@NandanaRaol can you confirm this works for you?

docs/releases/5.2.4.txt

sarahboyce · 2025-06-16T14:44:38Z

Just a note that I have managed to confirm that after this change CompositePKFilterTests.test_filter_users_by_comments_subquery passes when ran against SQL Server.
This was previously failing with pyodbc.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC Driver 18 for SQL Server][SQL Server]An expression of non-boolean type specified in a context where a condition is expected, near ','. (4145) (SQLExecDirectW)")

sarahboyce

Thank you ⭐

django/db/models/fields/tuple_lookups.py

amine-za · 2025-06-16T16:15:01Z

👍👍

django/db/models/fields/tuple_lookups.py

nessita

Thank you @charettes, this looks great! 🌟

nessita · 2025-06-30T15:50:44Z

buildbot, test on oracle.

nessita · 2025-06-30T18:01:26Z

buildbot, test on oracle.

nessita · 2025-06-30T20:38:07Z

There is a failure occurring in Oracle CI. This test (which I tweaked slightly in a throwaway commit to be able to see the whole result):

    def test_query_does_not_mutate(self):
        """
        Recompiling the same subquery doesn't mutate it.
        """
        queryset = Friendship.objects.filter(to_friend__in=Person.objects.all())
        self.assertEqual(str(queryset.query), str(queryset.query))

fails the assertion because the first str call is:

 SELECT "foreign_object_friendship"."id",
       "foreign_object_friendship"."from_friend_country_id",
       "foreign_object_friendship"."from_friend_id",
       "foreign_object_friendship"."to_friend_country_id",
       "foreign_object_friendship"."to_friend_id"
FROM   "foreign_object_friendship"
WHERE  EXISTS(SELECT 1 AS "A"
              FROM   "foreign_object_person"
              WHERE  ( "foreign_object_friendship"."to_friend_country_id" = (
                                 "foreign_object_person"."person_country_id" )
                       AND "foreign_object_friendship"."to_friend_id" = (
                           "foreign_object_person"."id" ) )
              FETCH first 1 ROWS only)

and the second:

 SELECT "foreign_object_friendship"."id",
       "foreign_object_friendship"."from_friend_country_id",
       "foreign_object_friendship"."from_friend_id",
       "foreign_object_friendship"."to_friend_country_id",
       "foreign_object_friendship"."to_friend_id"
FROM   "foreign_object_friendship"
WHERE  EXISTS(SELECT 1 AS "A"
              FROM   "foreign_object_person"
              WHERE  ( "foreign_object_friendship"."to_friend_country_id" = (
                                 "foreign_object_person"."person_country_id" )
                       AND "foreign_object_friendship"."to_friend_id" =
                           ( "foreign_object_person"."id" )
                       AND "foreign_object_friendship"."to_friend_country_id" =
                           (
                           "foreign_object_person"."person_country_id" )
                       AND "foreign_object_friendship"."to_friend_id" = (
                           "foreign_object_person"."id" ) )
              FETCH first 1 ROWS only)

showing there is an extra:

                       AND "foreign_object_friendship"."to_friend_id" =
                           ( "foreign_object_person"."id" )
                       AND "foreign_object_friendship"."to_friend_country_id" =
                           (
                           "foreign_object_person"."person_country_id" )

in the second one.

@charettes I don't have the capacity to debug before tomorrow, would you have any idea from the top of your head of why is this occurring? feels like a lack of cloning somewhere.

charettes · 2025-06-30T20:41:21Z

Sorry for the trouble, there use to be cloning in place but when I removed the In.get_prep_lookup duplicated logic I removed it as well. I'll push the part that is missing.

nessita · 2025-06-30T20:52:18Z

Sorry for the trouble, there use to be cloning in place but when I removed the In.get_prep_lookup duplicated logic I removed it as well. I'll push the part that is missing.

Yeah, I know, sorry about that, it's so hard to properly review without a way to run locally. I've tried to have a working Oracle 19 setup locally but haven't succeeded.

nessita · 2025-06-30T20:57:17Z

@charettes no worries, I managed a way to reproduce. I'm adding an analogous test_query_does_not_mutate to the composite_pk tests, and this immediately fails in the CompositePKFilterTupleLookupFallbackTests with sqlite. I can fix it I think!

    def test_query_does_not_mutate(self):
        queryset = User.objects.filter(comments__in=Comment.objects.all())
        self.assertEqual(str(queryset.query), str(queryset.query))

…support. When native support for tuple lookups is missing in a DB backend, it can be emulated with an EXISTS clause. This is controlled by the backend feature flag "supports_tuple_lookups". The mishandling of subquery right-hand side in `TupleIn` (added to support `CompositePrimaryKey` in Refs django#373) was likely missed because the only core backend we test with the feature flag disabled (Oracle < 23.4) supports it natively. Thanks to Nandana Raol for the report, and to Sarah Boyce, Jacob Walls, and Natalia Bidart for reviews.

nessita · 2025-06-30T21:01:27Z

buildbot, test on oracle.

charettes force-pushed the ticket-36464 branch from 7c0c878 to afec689 Compare June 14, 2025 15:12

charettes marked this pull request as ready for review June 14, 2025 15:12

charettes force-pushed the ticket-36464 branch from afec689 to d13db39 Compare June 14, 2025 15:18

charettes force-pushed the ticket-36464 branch from d13db39 to 33f30da Compare June 15, 2025 04:12

jacobtylerwalls reviewed Jun 16, 2025

View reviewed changes

docs/releases/5.2.4.txt Outdated Show resolved Hide resolved

sarahboyce force-pushed the ticket-36464 branch from 33f30da to 15368be Compare June 16, 2025 14:49

sarahboyce approved these changes Jun 16, 2025

View reviewed changes

sarahboyce reviewed Jun 16, 2025

View reviewed changes

django/db/models/fields/tuple_lookups.py Outdated Show resolved Hide resolved

bewithgaurav mentioned this pull request Jun 25, 2025

FEAT: Support 5.2- Added composite primary key microsoft/mssql-django#462

Draft

sarahboyce reviewed Jun 27, 2025

View reviewed changes

django/db/models/fields/tuple_lookups.py Outdated Show resolved Hide resolved

charettes force-pushed the ticket-36464 branch 2 times, most recently from b21ab32 to a9fc176 Compare June 27, 2025 17:30

nessita force-pushed the ticket-36464 branch 2 times, most recently from f5bbf9f to 2bd06ea Compare June 30, 2025 14:54

nessita approved these changes Jun 30, 2025

View reviewed changes

nessita force-pushed the ticket-36464 branch from 8367abb to e96006d Compare June 30, 2025 21:01

nessita merged commit 192bc7a into django:main Jun 30, 2025
34 of 35 checks passed

Uh oh!

Fixed #36464 -- Fixed tuple in subquery lookup when missing native support. #19565

Fixed #36464 -- Fixed tuple in subquery lookup when missing native support. #19565

Conversation

charettes commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Trac ticket number

Branch description

Uh oh!

charettes commented Jun 14, 2025

Uh oh!

sarahboyce commented Jun 16, 2025

Uh oh!

Uh oh!

sarahboyce commented Jun 16, 2025

Uh oh!

sarahboyce left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amine-za commented Jun 16, 2025

Uh oh!

Uh oh!

nessita left a comment

Choose a reason for hiding this comment

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

charettes commented Jun 30, 2025

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

nessita commented Jun 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

charettes commented Jun 14, 2025 •

edited

Loading