Обсуждение: trivial designated initializers

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)

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

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

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 get_mxact_status_for_lock())".

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
> get_mxact_status_for_lock())".

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)