Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add 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(dot)kapila16(at)gmail(dot)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