Aleksander Alekseev <aleksan...@timescale.com> writes: > One thing I'm not 100% sure of is whether record_larger() should make > a copy of its arguments or the current implementation is safe.
I don't see any copying happening in, say, text_larger or numeric_larger, so this shouldn't need to either. Personally I'd write "record_cmp(fcinfo) > 0" rather than indirecting through record_gt. The way you have it is not strictly correct anyhow: you're cheating by not using DirectFunctionCall. Also, given that you're passing the fcinfo, there's no need to extract the arguments from it before that call. So it seems to me that code like if (record_cmp(fcinfo) > 0) PG_RETURN_HEAPTUPLEHEADER(PG_GETARG_HEAPTUPLEHEADER(0)); else PG_RETURN_HEAPTUPLEHEADER(PG_GETARG_HEAPTUPLEHEADER(1)); should do, and possibly save one useless detoast step. Or you could do if (record_cmp(fcinfo) > 0) PG_RETURN_DATUM(PG_GETARG_DATUM(0)); else PG_RETURN_DATUM(PG_GETARG_DATUM(1)); because really there's no point in detoasting at all. regards, tom lane