On Fri, Jul 22, 2022 at 4:21 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, Jul 20, 2022 at 4:57 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> Thanks for the patch, my comments from the initial review:
> 1) Since we have changed the macros to inline functions, should we
> change the function names similar to the other inline functions in the
> same file like: ClearBufferTag, InitBufferTag & BufferTagsEqual:
I have thought about it while doing so but I am not sure whether it is
a good idea or not, because before my change these all were macros
with 2 naming conventions so I just changed to inline function so why
to change the name.
> -#define BUFFERTAGS_EQUAL(a,b) \
> -( \
> - RelFileLocatorEquals((a).rlocator, (b).rlocator) && \
> - (a).blockNum == (b).blockNum && \
> - (a).forkNum == (b).forkNum \
> -)
> +static inline void
> +CLEAR_BUFFERTAG(BufferTag *tag)
> +{
> + tag->rlocator.spcOid = InvalidOid;
> + tag->rlocator.dbOid = InvalidOid;
> + tag->rlocator.relNumber = InvalidRelFileNumber;
> + tag->forkNum = InvalidForkNumber;
> + tag->blockNum = InvalidBlockNumber;
> +}
>
> 2) We could move this macros along with the other macros at the top of the file:
> +/*
> + * The freeNext field is either the index of the next freelist entry,
> + * or one of these special values:
> + */
> +#define FREENEXT_END_OF_LIST (-1)
> +#define FREENEXT_NOT_IN_LIST (-2)
Yeah we can do that.
> 3) typo thn should be then:
> + * can raise it as necessary if we end up with more mapped relations. For
> + * now, we just pick a round number that is modestly larger thn the expected
> + * number of mappings.
> + */
>
> 4) There is one whitespace issue:
> git am v10-0004-Widen-relfilenumber-from-32-bits-to-56-bits.patch
> Applying: Widen relfilenumber from 32 bits to 56 bits
> .git/rebase-apply/patch:1500: space before tab in indent.
> (relfilenumber)))); \
> warning: 1 line adds whitespace errors.
Okay, I will fix it.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com