On Fri, Sep 22, 2017 at 11:47 AM, Andres Freund <and...@anarazel.de> wrote:

> Hi,
>
> On 2017-09-20 18:26:50 +0530, amul sul wrote:
> > Patch 0007:
>

> Other than these concern, patch looks pretty reasonable to me.
>
> I'd appreciate if you could have a look at the new version as well.
>
>
​I have reviewed
​
the newer version
​, looks good to me​.

​
Here are the few cosmetic comments ​for the 0003 patch​​:

1
​.
 982 +       ct->tuple.t_data = (HeapTupleHeader)
 983 +           MAXALIGN(((char *) ct) + sizeof(CatCTup));

It would be nice if you add a comment for this address alignment​​
​.​


2
​.
 947     /*
 948 -    * If there are any out-of-line toasted fields in the tuple,
expand them
 949 -    * in-line.  This saves cycles during later use of the catcache
entry, and
 950 -    * also protects us against the possibility of the toast tuples
being
 951 -    * freed before we attempt to fetch them, in case of something
using a
 952 -    * slightly stale catcache entry.
 953      */

Empty comment block left in the CatalogCacheCreateEntry().

3
​.
​ ​
411 +/*
 412 + *     CatalogCacheCompareTuple
 413 + *
 414 + * Compare a tuple to the passed arguments.
 415 + */
 416 +static inline bool
 417 +CatalogCacheCompareTuple(const CatCache *cache, int nkeys,
 418 +                        const Datum *cachekeys,
 419 +                        const Datum *searchkeys)

​Need an adjust to the p
rolog comment
​.​

Regards,
Amul

Reply via email to