Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter TableAdd Column...) - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter TableAdd Column...)
Date
Msg-id CAA4eK1+7KU+-iHS7FWmNHDr_hRjrN9xVr0DarJuOmRXgUMrzpw@mail.gmail.com
Whole thread Raw
In response to Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add Column...)  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter TableAdd Column...)
List pgsql-hackers
On Sun, Jun 3, 2018 at 5:08 AM, Amit Kapila <amit.kapila16@gmail.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


pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: WAL prefetch
Next
From: Ashutosh Bapat
Date:
Subject: Re: Partitioning with temp tables is broken