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

On Sun, Jun 3, 2018 at 5:08 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> Some assorted comments:
>
> 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;
> +
>
> a. Extra space (empty line) between structure and comments seems
> unnecessary.
> b. The names of structure members seem little difficult to understand if you
> and or others also think so, can we rename them to something like
> (missingPresent, missingVal) or (attmissingPresent, attmissingVal) or
> something similar.
>
> Patch to address 1 and 2a is attached. What do you think about 2b?
>

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.

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