Opened 3 weeks ago

Closed 4 days ago

Last modified 4 days ago

#36915 closed Cleanup/optimization (invalid)

Add support for select_for_update for implicit transactions

Reported by: David Owned by: MANAS MADESHIYA
Component: Database layer (models, ORM) Version: 6.0
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

Currently the select_for_update method is only allowed inside explicit transactions which have been started using transaction.atomic.

However any DML SQL statements can create an "implicit transaction" in which it may be useful to get control on row-level locks.

What I would like to achieve is a way to perform the following statement (inspired by this SO answer):

UPDATE my_table
SET my_col = 1
WHERE id IN (
    SELECT id
    FROM my_table
    WHERE ...
    FOR UPDATE
);

As of now it is not possible to perform this kind of query:

MyModel.objects.filter(pk__in=MyModel.objects.filter(...).select_for_update()).update(my_col=...)
#   raises TransactionManagementError: select_for_update cannot be used outside of a transaction.

The same can be accomplished by forcing the transaction

with transaction.atomic():
    MyModel.objects.filter(pk__in=MyModel.objects.filter(...).select_for_update()).update(my_col=...)
#  works!

It would be useful to improve locks management in bulk-operations.

If it is too difficult to add this kind of support with the ORM it could, at least, be documented as a tip in the docs.

Change History (6)

comment:1 by jaffar Khan, 3 weeks ago

I think select_for_update() is working as it has to be because using select_for_update() in implicit transactions will not make sense as no atomic boundary is defined, django will not know when to lock the rows and when to release the lock. It is defined in docs https://docs.djangoproject.com/en/6.0/ref/models/querysets/#select-for-update as

Returns a queryset that will lock rows until the end of the transaction

Even if you redefine the select_for_update() it will lock the rows and immidiatley release it before any updates so technically it will behave as implicit transactions.

comment:2 by MANAS MADESHIYA, 4 days ago

Owner: set to MANAS MADESHIYA
Status: newassigned

comment:3 by MANAS MADESHIYA, 4 days ago

PR: https://github.com/django/django/pull/20773

Problem:
select_for_update() raises TransactionManagementError when used
as a subquery inside .update() even though the DML statement
creates an implicit transaction at the database level.

Example:
MyModel.objects.filter(

pkin=MyModel.objects.filter(...).select_for_update()

).update(my_col=...)
# Previously raised TransactionManagementError
# Now works correctly

Fix:
Modified the check in compiler.py to skip the error when
select_for_update is used as a subquery inside a DML statement
by adding "and not self.query.subquery" condition.

Changes:

  • django/db/models/sql/compiler.py
  • tests/select_for_update/tests.py

comment:5 by Jacob Walls, 4 days ago

Has patch: unset
Resolution: invalid
Status: assignedclosed

I think the docs adequately cover this.

comment:6 by MANAS MADESHIYA, 4 days ago

I have already worked on this ticket and have a
proof of concept ready.

The fix involves adding "and not self.query.subquery"
condition in compiler.py to allow select_for_update()
in subqueries of DML statements.

Previous PR: https://github.com/django/django/pull/20773

Please consider accepting this ticket so I can
reopen the PR.

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