Hi,
On 2018-06-06 11:17:33 -0400, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > On 6/6/18 09:06, Andres Freund wrote:
> >> FmgrInfo really *is* call-site dependent, no? fn_extra, fn_mcxt, fn_expr
> >> all are. I think it's more useful to view the FmgrInfo /
> >> FunctionCallInfo data split as the separation between per-callsite and
> >> per-call data. And I think it'd make much more sense to officially
> >> treat collation as the former.
>
> > I think there are really three sets of information: catalog lookup
> > information, per query/expression information (such as collation), and
> > per-call information. But we only have two places to put things, so it
> > might look a bit odd.
>
> > Nothing wrong with considering changes, of course.
>
> Well, I still say this is an ad-hoc, unprincipled semantics change.
> If we're going to buy into FmgrInfo being call site dependent,
> we should go all the way. Why not move nargs there (or get rid of it,
> isn't it redundant with fn_nargs?), and possibly the context pointer?
Currently fn_nargs and nargs aren't quite the same unfortunately. The
latter is different for functions with variable arguments. But it's
still per-callsite rather than per-call. I agree that we, if we do
anything, make a bigger redesign. I just had a handle on nargs, but not
really on fncollation...
I don't think context necessarily is per-callsite, we use it for things
like the trigger context which changes per-row. I think there's still a
fair argument to be made to change it to move it to FmgrInfo, but it's
not quite as clear semantically.
One way to address the performance issue of having to set all these up
on every call would be to introduce a 'flags' field to FmgrInfo. Unless
FMGR_{CONTEXT,RESULTINFO,...} bits are set in it, the corresponding
FunctionCallInfoData fields would have undefined values and not be set.
I'm doubtful that's worth it with just context, resultinfo getting that
treatment, but if we ever were to extend that...
Greetings,
Andres Freund