RE: Proposal: Make use of C99 designated initialisers fornulls/values arrays - Mailing list pgsql-hackers

From Smith, Peter
Subject RE: Proposal: Make use of C99 designated initialisers fornulls/values arrays
Date
Msg-id 201DD0641B056142AC8C6645EC1B5F62014B98E4E6@SYD1217
Whole thread Raw
In response to Re: Proposal: Make use of C99 designated initialisers fornulls/values arrays  (Andres Freund <andres@anarazel.de>)
Responses Re: Proposal: Make use of C99 designated initialisers fornulls/values arrays  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
Hi Hackers.

This submission seems to have stalled.

Please forgive this post - I am unsure if the submission process expects me to come to defence of my own patch for one
lastgasp, or if I am supposed to just sit back and watch it die a slow death of a thousand cuts. 

I thought this submission actually started out very popular, but then support slowly eroded, and currently seems headed
towardsa likely rejection. 

~

Anyway, here are my arguments:

(a) I recognise that on first glance, the {0} syntax might evoke a momentary "double-take" by the someone reading the
code.IMO this would only be experienced by somebody encountering {0} syntax for the very first time.  This is not an
reallyuncommon "pattern" (it's already elsewhere in PostreSQL code), and once you've seen it two or three times there
isno confusion what it is doing. 

(b) Because of (a) I don't really agree with the notion that it should be replaced by a macro to hide the C syntax. I
didtry adding various macros as suggested, but all that achieved was to was spin off another 20 emails debating the
macroformat. I thought any code committer/reviewer should have no trouble at all to understand standard C syntax. 

(c) It was never a goal of this submission that *all* memsets should be replaced by {0}. Sometimes {0} is more concise
andbetter IMO, but sometimes memset is a way more appropriate choice. This patch only replaces simple examples of
primitivetypes like the values[] and nulls[] arrays (which was a repeated pattern for many tuple operations). I think
anyconcern that {0} may not work for all other complex cases is a red-herring. When memset is better, then use memset. 

(d) Wishing for C99 syntax to be same as the simpler {} style of C++ is another red-herring. I can only use what is
officiallysupported. It is what it is. 

(e) The PostgreSQL miscellaneous coding conventions - https://www.postgresql.org/docs/current/source-conventions.html -
saysto avoid " intermingled declarations and code". This leads to some code where the variable declaration and the
initialization(e.g. memset 0 or memset false) code can be widely separated. It can be an easy source of mistakes to
assumea variable was already initialized when maybe it wasn't. This patch puts the initialization at the point of
declaration,and so eliminates this risk. Isn't that best practice? 

(f) I'm still a bit perplexed how can it be that removing 200 lines of unnecessary function calls is not considered a
goodthing to do? Are patches that only tidy up code generally not accepted? I don't know. 

~

That's all I have to say in support of my patch; it will live or it will die according to the community wish.

If nothing else, at least I've learned a new term - "bike shedding" :-)

Kind Regards.
---
Peter Smith
Fujitsu Australia



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: an OID >= 8000 in master
Next
From: Amit Kapila
Date:
Subject: Re: logical decoding : exceeded maxAllocatedDescs for .spill files