Re: Variable-length FunctionCallInfoData - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Variable-length FunctionCallInfoData
Date
Msg-id 20181215194430.mfynjg7qyoliqst5@alap3.anarazel.de
Whole thread Raw
In response to Re: Variable-length FunctionCallInfoData  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Variable-length FunctionCallInfoData
List pgsql-hackers
Hi,

On 2018-12-15 10:45:21 -0500, Tom Lane wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> > "Andres" == Andres Freund <andres@anarazel.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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: 'infinity'::Interval should be added
Next
From: Andres Freund
Date:
Subject: Re: 'infinity'::Interval should be added