Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter TableAdd Column...)

Somehow I missed this thread ...



On 06/14/2018 02:01 PM, Alvaro Herrera wrote:
> On 2018-Jun-14, Amit Kapila wrote:
>
>> On Sun, Jun 3, 2018 at 5:08 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> 2.
>>> +/*
>>> + * Structure used to represent value to be used when the attribute is not
>>> + * present at all in a tuple, i.e. when the column was created after the
>>> tuple
>>> + */
>>> +
>>> +typedef struct attrMissing
>>> +{
>>> +   bool        ammissingPresent;   /* true if non-NULL missing value exists
>>> */
>>> +   Datum       ammissing;      /* value when attribute is missing */
>>> +} AttrMissing;
>>> +
>> As nobody responded, it seems that the variable naming pointed above
>> is okay, but in any case, I think we should fix cosmetic changes
>> proposed.  I will commit the patch unless you or someone thinks that
>> we should change the name of the variable.
> We used to use prefixes for common struct members names to help
> disambiguate across members that would otherwise have identical names in
> different structs.  Our convention was to use _ as a separator.  This
> convention has been partially lost, but seems we can use it to good
> effect here, by renaming ammissingPresent to am_present and ammissing to
> am_missing (I would go as far as suggesting am_default or am_substitute
> or something like that).


am_present and am_value perhaps? I'm not dogmatic about it.




> BTW I think "the result stored" is correct English.
>

Yes, it certainly is.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services