ri_LockPKTuple misleading message
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=2da86c1ef9b5446e0e22c0b6a5846293e58d98e3
+ case TM_Deleted:
+ if (IsolationUsesXactSnapshot())
+ ereport(ERROR,
+ (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+ errmsg("could not serialize access due to concurrent update")));errmsg should be
errmsg("could not serialize access due to concurrent delete")));
?
ExecLockRows also has the same situation.
On Sat, Apr 25, 2026 at 19:53 jian he <jian.universality@gmail.com> wrote:
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=2da86c1ef9b5446e0e22c0b6a5846293e58d98e3 + case TM_Deleted: + if (IsolationUsesXactSnapshot()) + ereport(ERROR, + (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), + errmsg("could not serialize access due to concurrent update")));errmsg should be
errmsg("could not serialize access due to concurrent delete")));
?ExecLockRows also has the same situation.
I guess the existing wording may have been using "concurrent update" in the
broader sense of "concurrent modification" of the tuple, so I'm not sure
that it's just an oversight.
That said, "concurrent delete" is more precise for the TM_Deleted case, so
I'll change it in the code I committed. As for ExecLockRows(), I'll
leave that alone unless others think we should change that too.
- Amit
Show quoted text
On Sat, Apr 25, 2026 at 7:31 PM Amit Langote <amitlangote09@gmail.com> wrote:
On Sat, Apr 25, 2026 at 19:53 jian he <jian.universality@gmail.com> wrote:
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=2da86c1ef9b5446e0e22c0b6a5846293e58d98e3 + case TM_Deleted: + if (IsolationUsesXactSnapshot()) + ereport(ERROR, + (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), + errmsg("could not serialize access due to concurrent update")));errmsg should be
errmsg("could not serialize access due to concurrent delete")));
?ExecLockRows also has the same situation.
I guess the existing wording may have been using "concurrent update" in the broader sense of "concurrent modification" of the tuple, so I'm not sure that it's just an oversight.
That said, "concurrent delete" is more precise for the TM_Deleted case, so I'll change it in the code I committed. As for ExecLockRows(), I'll
leave that alone unless others think we should change that too.
I have a feeling we should also update ExecLockRows(), since the
TM_Deleted branches in other places seem to use the wording
"concurrent delete".
cc andres since he was the original author of this code.
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
- Amit
--
Regards
Junwang Zhao
On Sat, Apr 25, 2026 at 20:42 Junwang Zhao <zhjwpku@gmail.com> wrote:
On Sat, Apr 25, 2026 at 7:31 PM Amit Langote <amitlangote09@gmail.com>
wrote:On Sat, Apr 25, 2026 at 19:53 jian he <jian.universality@gmail.com>
wrote:
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=2da86c1ef9b5446e0e22c0b6a5846293e58d98e3
+ case TM_Deleted: + if (IsolationUsesXactSnapshot()) + ereport(ERROR, + (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), + errmsg("could not serialize access due to concurrent update")));errmsg should be
errmsg("could not serialize access due to concurrent delete")));
?ExecLockRows also has the same situation.
I guess the existing wording may have been using "concurrent update" in
the broader sense of "concurrent modification" of the tuple, so I'm not
sure that it's just an oversight.That said, "concurrent delete" is more precise for the TM_Deleted case,
so I'll change it in the code I committed. As for ExecLockRows(), I'll
leave that alone unless others think we should change that too.
I have a feeling we should also update ExecLockRows(), since the
TM_Deleted branches in other places seem to use the wording
"concurrent delete".cc andres since he was the original author of this code.
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
Ah, OK, then let's change both instances for consistency, unless Andres
remembers a reason not to.
Thanks Junwang for checking that.
- Amit
Hi,
On 2026-04-25 20:59:50 +0900, Amit Langote wrote:
On Sat, Apr 25, 2026 at 20:42 Junwang Zhao <zhjwpku@gmail.com> wrote:
On Sat, Apr 25, 2026 at 7:31 PM Amit Langote <amitlangote09@gmail.com>
I have a feeling we should also update ExecLockRows(), since the
TM_Deleted branches in other places seem to use the wording
"concurrent delete".cc andres since he was the original author of this code.
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
Ah, OK, then let's change both instances for consistency, unless Andres
remembers a reason not to.Thanks Junwang for checking that.
No, I can't see any reason for that. I assume it was a copy & paste error,
but it's hard to know this far back.
Greetings,
Andres Freund
Hi Andres,
On Sat, Apr 25, 2026 at 10:38 PM Andres Freund <andres@anarazel.de> wrote:
On 2026-04-25 20:59:50 +0900, Amit Langote wrote:
On Sat, Apr 25, 2026 at 20:42 Junwang Zhao <zhjwpku@gmail.com> wrote:
On Sat, Apr 25, 2026 at 7:31 PM Amit Langote <amitlangote09@gmail.com>
I have a feeling we should also update ExecLockRows(), since the
TM_Deleted branches in other places seem to use the wording
"concurrent delete".cc andres since he was the original author of this code.
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
Ah, OK, then let's change both instances for consistency, unless Andres
remembers a reason not to.Thanks Junwang for checking that.
No, I can't see any reason for that. I assume it was a copy & paste error,
but it's hard to know this far back.
Thanks for chiming in.
Here is a patch to fix both instances. I'll leave the ExecLockRows()
instances unchanged in the back-branches due to the lack of user
complaints.
--
Thanks, Amit Langote
Attachments:
v1-0001-Use-concurrent-delete-in-serialization-error-for-.patchapplication/octet-stream; name=v1-0001-Use-concurrent-delete-in-serialization-error-for-.patchDownload+8-9
On Mon, Apr 27, 2026 at 1:51 PM Amit Langote <amitlangote09@gmail.com> wrote:
On Sat, Apr 25, 2026 at 10:38 PM Andres Freund <andres@anarazel.de> wrote:
On 2026-04-25 20:59:50 +0900, Amit Langote wrote:
On Sat, Apr 25, 2026 at 20:42 Junwang Zhao <zhjwpku@gmail.com> wrote:
On Sat, Apr 25, 2026 at 7:31 PM Amit Langote <amitlangote09@gmail.com>
I have a feeling we should also update ExecLockRows(), since the
TM_Deleted branches in other places seem to use the wording
"concurrent delete".cc andres since he was the original author of this code.
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
Ah, OK, then let's change both instances for consistency, unless Andres
remembers a reason not to.Thanks Junwang for checking that.
No, I can't see any reason for that. I assume it was a copy & paste error,
but it's hard to know this far back.Thanks for chiming in.
Here is a patch to fix both instances. I'll leave the ExecLockRows()
instances unchanged in the back-branches due to the lack of user
complaints.
New version where I added a test case to the isolation suite that
exercises ri_LockPKTuple().
Will push barring objections.
--
Thanks, Amit Langote