trivial designated initializers

  • Jump to comment-1
    Álvaro Herrera<alvherre@kurilemu.de>
    Jan 28, 2026, 12:21 PM UTC
    Hi
    We use C99 designated struct initializers in many places, but for some
    reason we don't do it in the tupleLockExtraInfo array in heapam.c nor in
    InternalBGWorkers array in bgworker.c. I've had this trivial patch
    rotting in a worktree for a long time. Any opposition to this change?
    Thanks,
    --
    Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
    "Postgres is bloatware by design: it was built to house
    PhD theses." (Joey Hellerstein, SIGMOD annual conference 2002)
    • Jump to comment-1
      Peter Eisentraut<peter@eisentraut.org>
      Jan 29, 2026, 12:04 AM UTC
      On 28.01.26 13:20, Álvaro Herrera wrote:
      { / LockTupleKeyShare /
      - AccessShareLock,
      - MultiXactStatusForKeyShare,
      - -1 / KeyShare does not allow updating tuples /
      + .hwlock = AccessShareLock,
      + .lockstatus = MultiXactStatusForKeyShare,
      + / KeyShare does not allow updating tuples /
      + .updstatus = -1
      },
      You could spruce this up further like
      [LockTupleKeyShare] = {
      .hwlock = AccessShareLock,
      ...
      },
      ...
      The comments "/ KeyShare does not allow updating tuples /" etc. seem repetitive and don't actually explain why -1 is an appropriate value. You could instead write a comment by the declaration of the updstatus field, like "set to -1 if the tuple lock mode does not allow updating tuples (see getmxactstatusforlock())".
      • Jump to comment-1
        Álvaro Herrera<alvherre@kurilemu.de>
        Jan 30, 2026, 9:28 AM UTC
        On 2026-Jan-29, Peter Eisentraut wrote:
        You could spruce this up further like

        [LockTupleKeyShare] = {
        .hwlock = AccessShareLock,
        ...
        },
        Oh right, done that way.
        The comments "/ KeyShare does not allow updating tuples /" etc. seem
        repetitive and don't actually explain why -1 is an appropriate value. You
        could instead write a comment by the declaration of the updstatus field,
        like "set to -1 if the tuple lock mode does not allow updating tuples (see
        getmxactstatusforlock())".
        Good point. I rewrote the comment on top of the declaration and pushed,
        thanks for the reviews.
        --
        Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
        "Hay dos momentos en la vida de un hombre en los que no debería
        especular: cuando puede permitírselo y cuando no puede" (Mark Twain)
    • Jump to comment-1
      Melanie Plageman<melanieplageman@gmail.com>
      Jan 28, 2026, 3:28 PM UTC
      On Wed, Jan 28, 2026 at 7:21 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:

      We use C99 designated struct initializers in many places, but for some
      reason we don't do it in the tupleLockExtraInfo array in heapam.c nor in
      InternalBGWorkers array in bgworker.c. I've had this trivial patch
      rotting in a worktree for a long time. Any opposition to this change?
      I find these much easier to understand with the designated
      initializers (and I am a big fan of designated initializers in
      general). So +1
      - Melanie
      • Jump to comment-1
        Jelte Fennema-Nio<postgres@jeltef.nl>
        Jan 28, 2026, 10:48 PM UTC
        On Wed, 28 Jan 2026 at 16:28, Melanie Plageman
        <melanieplageman@gmail.com> wrote:
        I find these much easier to understand with the designated
        initializers (and I am a big fan of designated initializers in
        general). So +1
        yes, +1