On 03/11/22 22:18, Paul Jungwirth wrote: > Arg, fixed. > >> In range_agg_transfn, you've changed the message in the "must be called >> with a range or multirange"; that seems like another good candidate to >> be an elog. > > Agreed. Updated here.
This looks good to me and passes installcheck-world, so I'll push the RfC button. > Sharing a prosrc seems even less remarkable than sharing an aggfinalfn. > You're right there are no cases for other finalfns yet, but I don't think > there is anything special about finalfns that would make this a weirder > thing to do there than with ordinary functions. You sent me back to look at how many of those there are. I get 42 cases of shared prosrc (43 now). The chief subgroup of those looks to involve sharing between parameter signatures where the types have identical layouts and the semantic differences are unimportant to the function in question (comparisons between bit or between varbit, overlaps taking timestamp or timestamptz, etc.). The other prominent group is range and multirange constructors, where the C function has an obviously generic name like range_constructor2 and gets shared by a bunch of SQL declarations. I think here we've added the first instance where the C function is shared by SQL-declared functions accepting two different polymorphic pseudotypes. But it's clearly simple and works, nothing objectionable about it. I had experimented with renaming multirange_agg_finalfn to just range_agg_finalfn so it would just look like two overloads of one function sharing a prosrc, and ultimately gave up because genbki.pl couldn't resolve the OID where the name is used in pg_aggregate.dat. That's why it surprised me to see three instances where other functions (overlaps, isfinite, name) do use the same SQL name for different overloads, but the explanation seems to be that nothing else at genbki time refers to those, so genbki's unique-name limitation doesn't affect them. Neither here nor there for this patch, but an interesting new thing I learned while reviewing it. Regards, -Chap
