pgsql-hackers
❮
trivial designated initializers
- Jump to comment-1Álvaro Herrera<alvherre@kurilemu.de>Jan 28, 2026, 12:21 PM UTCHi
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-1Peter Eisentraut<peter@eisentraut.org>Jan 29, 2026, 12:04 AM UTCOn 28.01.26 13:20, Álvaro Herrera wrote:
{ / LockTupleKeyShare /
You could spruce this up further like
- AccessShareLock,
- MultiXactStatusForKeyShare,
- -1 / KeyShare does not allow updating tuples /
+ .hwlock = AccessShareLock,
+ .lockstatus = MultiXactStatusForKeyShare,
+ / KeyShare does not allow updating tuples /
+ .updstatus = -1
},
[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 UTCOn 2026-Jan-29, Peter Eisentraut wrote:
You could spruce this up further like
Oh right, done that way.
[LockTupleKeyShare] = {
.hwlock = AccessShareLock,
...
},The comments "/ KeyShare does not allow updating tuples /" etc. seem
Good point. I rewrote the comment on top of the declaration and pushed,
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())".
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-1Melanie Plageman<melanieplageman@gmail.com>Jan 28, 2026, 3:28 PM UTCOn Wed, Jan 28, 2026 at 7:21 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
I find these much easier to understand with the designated
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?
initializers (and I am a big fan of designated initializers in
general). So +1
- Melanie- Jump to comment-1Jelte Fennema-Nio<postgres@jeltef.nl>Jan 28, 2026, 10:48 PM UTCOn Wed, 28 Jan 2026 at 16:28, Melanie Plageman
<melanieplageman@gmail.com> wrote:I find these much easier to understand with the designated
yes, +1
initializers (and I am a big fan of designated initializers in
general). So +1