On Mon, 6 Nov 2023 at 17:07, Alexander Korotkov <aekorot...@gmail.com> wrote:
>
> Hi!
>
> On Wed, Jul 5, 2023 at 4:46 PM Aleksander Alekseev <aleksan...@timescale.com>
> wrote:
> > PFE the corrected patchset v58.
>
> I'd like to revive this thread.
>
> This patchset is extracted from a larger patchset implementing 64-bit xids.
> It converts page numbers in SLRUs into 64 bits. The most SLRUs save the same
> file naming scheme, thus their on-disk representation remains the same.
> However, the patch 0002 converts pg_notify to actually use a new naming
> scheme. Therefore pg_notify can benefit from simplification and getting rid
> of wraparound.
>
> -#define TransactionIdToCTsPage(xid) \
> - ((xid) / (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
> +
> +/*
> + * Although we return an int64 the actual value can't currently exceeed
> 2**32.
> + */
> +static inline int64
> +TransactionIdToCTsPage(TransactionId xid)
> +{
> + return xid / (int64) COMMIT_TS_XACTS_PER_PAGE;
> +}
>
> Is there any reason we transform macro into a function? If not, I propose to
> leave this as a macro. BTW, there is a typo in a word "exceeed".
If I remember right, the compiler will make equivalent code from
inline functions and macros, and functions has an additional benefit:
the compiler will report type mismatch if any. That was the only
reason.
Also, I looked at v58-0001 and don't quite agree with mentioning the
authors of the original 64-xid patch, from which the current patch is
derived, as just "privious input" persons.
Regards, Pavel Borisov
