From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Variable-length FunctionCallInfoData |
Date: | 2018-12-15 19:44:30 |
Message-ID: | 20181215194430.mfynjg7qyoliqst5@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2018-12-15 19:48:55 | Re: 'infinity'::Interval should be added |
Previous Message | Tom Lane | 2018-12-15 19:43:49 | Re: 'infinity'::Interval should be added |