On 11/05/2016 01:13 PM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 11/05/2016 11:46 AM, Tom Lane wrote:
>>> That may be a good fix for robustness purposes, but it seems pretty horrid
>>> from an efficiency standpoint. Where is this call, and should we be
>>> modifying it to provide a flinfo?
>> I thought of providing an flinfo, but I couldn't see a simple way to do
>> it that would provide something much longer lived than the function
>> call, in which case it seemed a bit pointless. That's why I asked for
>> assistance :-)
> Hmm. The problem is that the intermediate layer in btree_gist (and
> probably btree_gin too, didn't look) does not pass through the
> FunctionCallInfo data structure that's provided by the GIST AM.
> That wasn't needed up to now, because none of the supported data types
> are complex enough that any cached state would be useful, but trying
> to extend it to enums exposes the shortcoming.
>
> We could fix this, but it would be pretty invasive since it would require
> touching just about every function in btree_gist/gin. Not entirely sure
> that it's worth it. On the other hand, the problem might well come up
> again in future, perhaps for a datatype where the penalty for lack of
> a cache is greater --- eg, it would be pretty painful to support
> record_cmp without caching. And the changes ought to be relatively
> mechanical, even if they are extensive.
Yeah. I think it's probably worth doing. btree_gin is probably a better
place to start because it's largely macro-ized.
I don't have time right now to do this. I'll try to get to it if nobody
else does over then next couple of months.
>
> BTW, this patch would be a great deal shorter if you made use of the
> work done in 40b449ae8. That is, you no longer need to replace the
> base extension script --- just add an update script and change the
> default version in the .control file. See fd321a1df for an example.
Oh, nice. My work was originally done in March, before this came in.
cheers
andrew