On 16.05.22 15:23, Amul Sul wrote:
> +static inline OffsetNumber
> +PageGetMaxOffsetNumber(Page page)
> +{
> + if (((PageHeader) page)->pd_lower <= SizeOfPageHeaderData)
> + return 0;
> + else
> + return ((((PageHeader) page)->pd_lower - SizeOfPageHeaderData)
> / sizeof(ItemIdData));
> +}
>
> The "else" is not necessary, we can have the return statement directly
> which would save some indentation as well. The Similar pattern can be
> considered for 0004 and 0007 patches as well.
I kind of like it better this way. It preserves the functional style of
the original macro.
> +static inline void
> +XLogFromFileName(const char *fname, TimeLineID *tli, XLogSegNo
> *logSegNo, int wal_segsz_bytes)
> +{
> + uint32 log;
> + uint32 seg;
> + sscanf(fname, "%08X%08X%08X", tli, &log, &seg);
> + *logSegNo = (uint64) log * XLogSegmentsPerXLogId(wal_segsz_bytes) + seg;
> +}
>
> Can we have a blank line after variable declarations that we usually have?
done
> 0006 patch:
> +static inline Datum
> +fetch_att(const void *T, bool attbyval, int attlen)
> +{
> + if (attbyval)
> + {
> +#if SIZEOF_DATUM == 8
> + if (attlen == sizeof(Datum))
> + return *((const Datum *) T);
> + else
> +#endif
>
> Can we have a switch case like store_att_byval() instead of if-else,
> code would look more symmetric, IMO.
done
