Use "NO KEY" when doing select_for_update for PostgreSQL

[Manuel Weitzman]

Currently Django compiles select for update into one of

• FOR UPDATE
• FOR UPDATE NOWAIT
• FOR UPDATE SKIP LOCKED

All of these acquire a lock which conflicts with KEY SHARE locks. This means that updates on tables which references a row locked by FOR UPDATE will have to wait until the lock is released. This is to avoid any unexpected changes on PRIMARY KEY fields but, in Django, primary key fields are read-only, so locking them makes no sense.

There is no need to acquire these kind of locks, instead, we should (at least be able to) use

• FOR NO KEY UPDATE
• FOR NO KEY UPDATE NOWAIT
• FOR NO KEY UPDATE SKIP LOCKED

which can be easily done by overriding BaseOperation.for_update_sql in Postgres DatabaseOperation

For 1.11

def for_update_sql(self, nowait=False, skip_locked=False):
    """
    Returns the FOR UPDATE SQL clause to lock rows for an update operation.
    """
    if nowait:
        return 'FOR NO KEY UPDATE NOWAIT'
    elif skip_locked:
        return 'FOR NO KEY UPDATE SKIP LOCKED'
    else:
        return 'FOR NO KEY UPDATE'

For 2.2

def for_update_sql(self, nowait=False, skip_locked=False, of=()):
    """
    Return the FOR UPDATE SQL clause to lock rows for an update operation.
    """
    return 'FOR NO KEY UPDATE%s%s%s' % (
        ' OF %s' % ', '.join(of) if of else '',
        ' NOWAIT' if nowait else '',
        ' SKIP LOCKED' if skip_locked else '',
    )

Not doing so causes lock contention in our use case.

[Simon Charette]

... in Django, primary key fields are read-only, so locking them makes no sense

That's not true AFAIK. Even if AutoField are backed by sequences and generated on the database side they can still be overridden by developers. Same thing with non-AutoField primary keys (e.g. UUIDField(default=uuid.uuid4)).

On the other hand updating a primary key while holding a lock is really uncommon so I wouldn't be against adding a select_for_udate(primary_key) parameter that defaults to False. We just have to make sure that we still allow these operations to be performed somehow.

[Manuel Weitzman]

Replying to Simon Charette:

... in Django, primary key fields are read-only, so locking them makes no sense

That's not true AFAIK. Even if AutoField are backed by sequences and generated on the database side they can still be overridden by developers. Same thing with non-AutoField primary keys (e.g. UUIDField(default=uuid.uuid4)).

Then the docs are wrong or at least misleading: "The primary key field is read-only. If you change the value of the primary key on an existing object and then save it, a new object will be created alongside the old one."" (​https://docs.djangoproject.com/en/2.2/topics/db/models/)

Which means that UPDATEs over PKs are transformed into INSERTs

On the other hand updating a primary key while holding a lock is really uncommon so I wouldn't be against adding a select_for_udate(primary_key) parameter that defaults to False. We just have to make sure that we still allow these operations to be performed somehow.

That is possible, I tested today and only a few changes are required to achieve this behavior. But if UPDATEs are converted to INSERTs, then NO KEY should be used by default when using a PostgreSQL backend, shouldn't it?
Maybe I could send a PR soon for review, I think I can program an approach which uses an extra parameter!

[Simon Charette]

The docs are correct for save but primary keys are still updatable through queryset.filter(pk=old_value).update(pk=new_value).

Feel free to submit a PR. It'd be great to investigate if other backends have a similar option as well.

[Mads Jensen]

This needs to be in the review queue :)

[Mariusz Felisiak <felisiak.mariusz@…>]

In a4e6030:

Fixed #30375 -- Added FOR NO KEY UPDATE support to QuerySet.select_for_update() on PostgreSQL.