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