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

On Fri, Jun 15, 2018 at 12:54 AM, Andrew Dunstan
<andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
>
> 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;
>>>> +
..
>>
>> 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.
>

+1. Attached patch changed the names as per suggestion.

>
>
>
>> BTW I think "the result stored" is correct English.
>>
>
> Yes, it certainly is.
>

Okay.

How about attached?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com