Re: Variable-length FunctionCallInfoData

Hi,

On 2018-12-15 10:45:21 -0500, Tom Lane wrote:
> Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> > "Andres" == Andres Freund <andres(at)anarazel(dot)de> writes:
> >> I think it'd probably good to add accessors for value/nullness in
> >> arguments that hide the difference between <v12 and v12, for the
> >> sake of extension authors. Would probably mostly make sense if we
> >> backpatched those for compatibility.
>
> > Speaking as an affected extension author: don't backpatch them.
>
> Yeah, I agree.

Ok, convinced.

> Looking at the patch, it seems like a real pain that substituting
> "STACK_FCINFO_FOR_ARGS(fcinfo, ...)" for "FunctionCallInfoData fcinfo"
> has the effect of converting "fcinfo" into a pointer. Is there a way
> to define the macro so that that doesn't happen, and the ensuing
> minor-but-invasive code changes aren't needed? (It'd be easy in C++,
> but not quite seeing how to do it in C, at least not without defining
> an additional macro for "fcinfo" that we'd then need to #undef at the
> end of the function.)

It'd be nice, but I wasn't able to come up with anything either. While
I'd like that, I'm unfortunately doubtful others will agree to move to
c++ for this :P.

> 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.

> 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.

> 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.

> I do not think the 0002 patch is a good idea. It's going to add
> cycles to function calls, and it's not buying anything I'd call
> important.

Yea, same conclusion I came to. I dislike those verbose copies of
functions a lot, but the approach I could find out to resolve that, seem
like a cure worse than the disease.

Greetings,

Andres Freund