LockMode::NONE should not require transaction? #6926

Open
opened 2026-01-22 15:41:31 +01:00 by admin · 1 comment
Owner

Originally created by @fliespl on GitHub (Feb 10, 2022).

Bug Report

Q A
BC Break yes
Version 2.11

Summary

Up for discussion if LockMode::NONE is really a transactional items.

Current behavior

Currently setting setLockMode(LockMode::NONE) does check for valid transaction, even though it doesn't do anything useful with most (all?) database servers.

Additionally, there is a check, which should be removed, since strict rules say that setLockMode should be int (phpstan returns errors when calling setLockMode(null/false).

        if ($lockMode === null || $lockMode === false || $lockMode === LockMode::NONE) {
            return $sql;
        }

How to reproduce

            $query->setLockMode($forUpdate ? LockMode::PESSIMISTIC_WRITE : LockMode::NONE);

Expected behavior

Allow non-transaction usage.

Originally created by @fliespl on GitHub (Feb 10, 2022). ### Bug Report | Q | A |------------ | ------ | BC Break | yes | Version | 2.11 #### Summary Up for discussion if LockMode::NONE is really a transactional items. #### Current behavior Currently setting setLockMode(LockMode::NONE) does check for valid transaction, even though it doesn't do anything useful with most (all?) database servers. Additionally, there is a check, which should be removed, since strict rules say that setLockMode should be int (phpstan returns errors when calling `setLockMode(null/false)`. ``` if ($lockMode === null || $lockMode === false || $lockMode === LockMode::NONE) { return $sql; } ``` #### How to reproduce ``` $query->setLockMode($forUpdate ? LockMode::PESSIMISTIC_WRITE : LockMode::NONE); ``` #### Expected behavior Allow non-transaction usage.
admin added the Bug label 2026-01-22 15:41:31 +01:00
Author
Owner

@hubertnnn commented on GitHub (Apr 11, 2023):

I just wanted to add the same issue.
There is one extra thing that I would suggest.

After checking if setLockMode(LockMode::NONE) actually does nothing I found that HINT_LOCK_MODE that is set by this function is used in exactly 3 places:

  • SqlWalker.php line 546 $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE;
  • SqlWalker.php line 970 $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE
  • Query.php line 795 Query::getLockMode() function

In first two LockMode::NONE is the default, in the third place null is default.
It also seems that Query::getLockMode() is unused (at least on my project)

So it seems that there is mostly no difference between not setting LockMode and setting it to LockMode::NONE
I would suggest not only removing the exception for transactions
but also improving consistency by returning LockMode::NONE from Query::getLockMode() as default.

edit:
Also until this is fixed a wourkaround would be to use false instead of LockMode::NONE since its effects are the same, but false will not throw an exception.

$query->setLockMode($forUpdate ? LockMode::PESSIMISTIC_WRITE : false);
@hubertnnn commented on GitHub (Apr 11, 2023): I just wanted to add the same issue. There is one extra thing that I would suggest. After checking if `setLockMode(LockMode::NONE)` actually does nothing I found that `HINT_LOCK_MODE` that is set by this function is used in exactly 3 places: - `SqlWalker.php` line 546 `$this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE;` - `SqlWalker.php` line 970 `$this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE` - `Query.php` line 795 `Query::getLockMode()` function In first two `LockMode::NONE` is the default, in the third place `null` is default. It also seems that `Query::getLockMode()` is unused (at least on my project) So it seems that there is mostly no difference between not setting LockMode and setting it to `LockMode::NONE` I would suggest not only removing the exception for transactions but also improving consistency by returning `LockMode::NONE` from `Query::getLockMode()` as default. edit: Also until this is fixed a wourkaround would be to use `false` instead of `LockMode::NONE` since its effects are the same, but `false` will not throw an exception. ``` $query->setLockMode($forUpdate ? LockMode::PESSIMISTIC_WRITE : false); ```
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6926