[Django] #35836: ORM can produce invalid SQL when subclassing Query

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------
Reporter: Ben Pearman | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.0 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
We have observed a bug in the ORM where invalid SQL can be produced.

A reproduction can be seen here:
https://github.com/benpearman/django-orm-bug
And specifically:
https://github.com/benpearman/django-orm-bug/blob/master/demo/models.py

It is caused with the following steps:
1. Subclassing {{{django.db.models.sql.Query}}} and using it in a Queryset
mixin
2. Creating a queryset with an {{{exclude()}}} expression which includes a
subquery. EG {{{Classroom.objects.exclude(student_set__id=student_A.id)}}}
3. Then before calling {{{super().get_compiler}}} from our subclassed
Query:
- Clone the query
- Adding a simple expression via {{{cloned_query.add_q}}} eg
{{{cloned_query.add_q(Q(school_id=1))}}}
- Passing the cloned query into {{{super().get_compiler}}}
4. Then evaluate the queryset, which produces invalid SQL and raises
{{{django.db.utils.OperationalError}}}

Note the issue happens when get_compiler is invoked for the subquery, not
on the main query itself.

This was working for us in Django 3.2, but not for Django 4.1

git bisect shows the breakage was introduced by this commit:
https://github.com/django/django/commit/14c8504a37afad96ab93cf82f47b13bcc4d00621
--
Ticket URL: <https://code.djangoproject.com/ticket/35836>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------

Reporter: Ben Pearman | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Craig de Stigter):

* cc: Craig de Stigter (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/35836#comment:1>

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------
Reporter: Ben Pearman | Owner: (none)

Type: Bug | Status: closed

Component: Database layer | Version: 5.0
(models, ORM) |

Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* resolution: => invalid
* status: new => closed

Comment:

Django 3.2 has been EOL for 6 months now and while we accept patch to
allow extension of the ORM we don't offer any guarantees towards backward
compatibility around `sql.Query` API changes.

In other words, if you extend undocumented and unstable part of the
framework you must be prepared to adapt your code and provide a solution
that goes beyond bisecting the origin of your breakage.

In the case of 14c8504a37afad96ab93cf82f47b13bcc4d00621 I think it's a
sane change in towards extensibility and something we'd want to keep in
`main`. `SQLQuery.get_compiler` is by no means meant to be used as a hook
to augment the where clause of a `QuerySet`, this should be done at the
manager level.

At this point, even if we were to land a solution that you'd have to
provide and build a rationale for it wouldn't make it to a public release
until 5.2 which is meant to be released in April 2025. If your solution
runs into problems when dealing with subqueries and you want to double
down on this approach you should be able to identify the proper attributes
to used specialized logic.
--
Ticket URL: <https://code.djangoproject.com/ticket/35836#comment:2>

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------
Reporter: Ben Pearman | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Craig de Stigter):

Thanks, suspected as much.

We'll follow up with a django-users post about how to implement something
to support our use-case. (as far as we can tell, there isn't any way to do
it using the supported APIs)
--
Ticket URL: <https://code.djangoproject.com/ticket/35836#comment:3>

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------
Reporter: Ben Pearman | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

You might want to use [https://forum.djangoproject.com/c/users/using-the-
orm/17 the forums for that] as they are more active nowadays.
--
Ticket URL: <https://code.djangoproject.com/ticket/35836#comment:4>

[Django]

#35836: ORM can produce invalid SQL when subclassing Query
-------------------------------------+-------------------------------------
Reporter: Ben Pearman | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Ben Pearman):

Thanks for your detailed answer Simon. We'll followup on the forums as
well.
--
Ticket URL: <https://code.djangoproject.com/ticket/35836#comment:5>