On Sun, Jun 3, 2018 at 5:08 AM, Amit Kapila <amit.kapil...@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