Opened 50 minutes ago
#37432 new Bug
QuerySet.select_for_update() after union() and friends is silently ignored
| Reported by: | Alex Henman | Owned by: | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
select_for_update() doesn't raise an error when called after union(), intersection() or difference(). Instead, the locking is silently dropped:
- the generated SQL contains no FOR UPDATE clause, so no rows are locked;
- the "select_for_update cannot be used outside of a transaction."
TransactionManagementErroris never raised, because the compiler only performs that check for non-combined queries.
For example, on PostgreSQL:
from django.db import transaction qs = Number.objects.filter(num__lt=5) qs.select_for_update() # outside a transaction, as expected: TransactionManagementError: select_for_update cannot be used outside of a transaction. qs.union(qs).select_for_update() # no error, query runs <QuerySet [<Number: 0>, <Number: 4>, <Number: 3>, <Number: 2>, <Number: 1>]> with transaction.atomic(): print(qs.union(qs).select_for_update().query) # NOTE: there's no FOR UPDATE (SELECT "queries_number"."id" AS "col1", "queries_number"."num" AS "col2", "queries_number"."other_num" AS "col3", "queries_number"."another_num" AS "col4" FROM "queries_number" WHERE "queries_number"."num" < 5) UNION (SELECT "queries_number"."id" AS "col1", "queries_number"."num" AS "col2", "queries_number"."other_num" AS "col3", "queries_number"."another_num" AS "col4" FROM "queries_number" WHERE "queries_number"."num" < 5)
The union() docs say that only slicing, count(), exists(), order_by(), and values()/values_list() are allowed on the resulting QuerySet. Other unsupported methods already raise NotSupportedError ("Calling QuerySet.filter() after union() is not supported.") following #27995, #31148, and #31580, but select_for_update was never added to that list. PostgreSQL rejects FOR UPDATE on UNION/INTERSECT/EXCEPT and their inputs, and Oracle and SQLite don’t support it either. So raising NotSupportedError like the other methods seems the right fix. I think that’s a one-line self._not_support_combined_queries("select_for_update") in QuerySet.select_for_update().
Here's an example failing test based on the others in test_qs_combinators:
def test_select_for_update_unsupported_on_combined_qs(self): qs = Number.objects.all() msg = "Calling QuerySet.select_for_update() after %s() is not supported." combinators = ["union"] if connection.features.supports_select_difference: combinators.append("difference") if connection.features.supports_select_intersection: combinators.append("intersection") for combinator in combinators: with self.subTest(combinator=combinator): with self.assertRaisesMessage(NotSupportedError, msg % combinator): getattr(qs, combinator)(qs).select_for_update()
I had a hunt for any other methods that might be missing this check and the only one flagged up was the internal complex_filter. I'm not really familiar with this method but it would be simple enough to add the same check there if it's considered necessary.
Happy to create the patch as outlined above if that seems like a sensible fix.