postgres.email
❮

ri_LockPKTuple misleading message

  • Jump to comment-1
    jian he<jian.universality@gmail.com>
    Apr 25, 2026, 10:53 AM UTC
    Hi.
    https://git.postgresql.org/cgit/postgresql.git/commit/?id=2da86c1ef9b5446e0e22c0b6a5846293e58d98e3
    + case TM_Deleted:
    + if (IsolationUsesXactSnapshot())
    + ereport(ERROR,
    + (errcode(ERRCODETRSERIALIZATIONFAILURE),
    + 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.
    --
    jian
    https://www.enterprisedb.com/
    View in PostgreSQL Archives →
    • Jump to comment-1
      Amit Langote<amitlangote09@gmail.com>
      Apr 25, 2026, 11:31 AM UTC
      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(ERRCODETRSERIALIZATIONFAILURE),
      + 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
      View in PostgreSQL Archives →
      • Jump to comment-1
        Junwang Zhao<zhjwpku@gmail.com>
        Apr 25, 2026, 11:42 AM UTC
        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(ERRCODETRSERIALIZATIONFAILURE),
        + 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
        View in PostgreSQL Archives →
        • Jump to comment-1
          Amit Langote<amitlangote09@gmail.com>
          Apr 25, 2026, 12:00 PM UTC
          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(ERRCODETRSERIALIZATIONFAILURE),
          + 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
          https://github.com/postgres/postgres/blob/REL_12_STABLE/src/backend/executor/nodeLockRows.c#L230
          View in PostgreSQL Archives →
          • Jump to comment-1
            Andres Freund<andres@anarazel.de>
            Apr 25, 2026, 1:38 PM UTC
            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
            View in PostgreSQL Archives →
            • Jump to comment-1
              Amit Langote<amitlangote09@gmail.com>
              Apr 27, 2026, 4:52 AM UTC
              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
              View in PostgreSQL Archives →
              • Jump to comment-1
                Amit Langote<amitlangote09@gmail.com>
                Apr 28, 2026, 5:59 AM UTC
                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
                View in PostgreSQL Archives →