Hi, > 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.
Many thanks. Here is the corrected patch. Now it also includes MIN() support and tests. -- Best regards, Aleksander Alekseev
v2-0001-Support-min-record-and-max-record-aggregates.patch
Description: Binary data