On 02/23/2017 08:34 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On 02/23/2017 04:41 PM, Tom Lane wrote:
>>> BTW, while I'm looking at this ... isn't gin_enum_cmp broken on its face?
>> Yes, that's what I'm trying to fix.
> I'd forgotten where this thread started. For a minute there I thought
> we had a live bug, rather than a deficiency in code-under-development.
>
> After thinking about that, I believe that enum_cmp_internal is a rather
> considerable hazard. It might not be our only function that requires
> fcinfo->flinfo cache space just some of the time not all the time, but
> I don't recall anyplace else that could so easily undergo a reasonable
> amount of testing without *ever* reaching the case where it requires
> that cache space. So I'm now worried that it wouldn't be too hard for
> such a bug to get past us.
>
> I think we should address that by adding a non-bypassable Assert that
> the caller did provide flinfo, as in the attached.
>
>
Looks sane. I don't believe there is anywhere in the core code that
calls this without fcinfo->flinfo, But obviously I have tickled that
with my original patch for the extension.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services