Re: [PATCH] Add min() and max() aggregate functions for xid8

On 2022/02/05 10:46, Ken Kato wrote:
> Thank you for the comments.
> I sent my old version of patch by mistake.
> This is the updated one.

Thanks!

+	PG_RETURN_FULLTRANSACTIONID((FullTransactionIdFollowsOrEquals(fxid1, fxid2)) ? fxid1 : fxid2);

Basically it's better to use less 80 line length for readability. So how about change the format of this to the following?

if (FullTransactionIdFollows(fxid1, fxid2))
     PG_RETURN_FULLTRANSACTIONID(fxid1);
else
     PG_RETURN_FULLTRANSACTIONID(fxid2);

+insert into xid8_tab values ('0'::xid8), ('18446744073709551615'::xid8);

Isn't it better to use '0xffffffffffffffff'::xid8 instead of '18446744073709551615'::xid8, to more easily understand that this test uses maximum number allowed as xid8?

In addition to those two xid8 values, IMO it's better to insert also the xid8 value neither minimum nor maximum xid8 ones, for example, '42'::xid8.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION