Re: Variable-length FunctionCallInfoData

Hi,

On 2018-12-15 11:44:30 -0800, Andres Freund wrote:
> On 2018-12-15 10:45:21 -0500, Tom Lane wrote:
> > I also wonder if we should rename the type FunctionCallInfoData,
> > perhaps to FunctionCallInfo_Data, so as to intentionally break
> > code that hasn't been converted. On the other hand, that might
> > introduce too much useless code churn --- not sure how many live
> > references to that struct type will remain in place.
>
> Probably doable, there ought not to be many FunctionCallInfoData
> references afterwards.

I did not like FunctionCallInfo_Data, the _ grated
me. FunctionCallInfoBaseData isn't great, but seems better?

> > With or without that, I'm pretty sure you wanted the pad member to be
> > char fcinfo_data[SizeForFunctionCallInfoData(nargs)]; \
> > not
> > char *fcinfo_data[SizeForFunctionCallInfoData(nargs)]; \

Indeed. And that hid a bug or two, where not enough space for
arguments was allocated. I changed a few on-stack infos to be of
FUNC_MAX_ARGS length, because they're not known at compile time. Seems
better than unnecesarily introducing a dynamic allocation, and they're
not that hot locations.

> > One more naming thought: would "LOCAL_FCINFO(...)" be a better
> > name for that macro? I don't think FOR_ARGS is adding much in
> > any case.
>
> Hm, that works.

Done.

> > Why does struct agg_strict_input_check now have *both*
> > NullableDatum and "bool *nulls"? If that's not a typo,
> > it needs to be documented what the fields are for.
>
> I'll check whether it can be simplified.

It can't trivially: For tuplesort cases the null check points into
TupleTableSlot's isnull array, but for other aggs it points into
FunctionCallInfoData->args. If we change TupleTableSlots to use
NullableDatum as well - probably a good idea for efficiency reasons -
we could change that, but that's a separate reasonably large sided
patch. Added a comment to that end.

Updated patch attached. Besides the above changes, there's a fair bit
of comment changes, and I've implemented the necessary JIT changes.

Greetings,

Andres Freund