Re: [HACKERS] btree_gin and btree_gist for enums - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] btree_gin and btree_gist for enums
Date
Msg-id 8258.1487886101@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] btree_gin and btree_gist for enums  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
Responses Re: [HACKERS] btree_gin and btree_gist for enums  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
Re: [HACKERS] btree_gin and btree_gist for enums  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
Re: [HACKERS] btree_gin and btree_gist for enums  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
Re: [HACKERS] btree_gin and btree_gist for enums  (Emre Hasegeli <emre@hasegeli.com>)
List pgsql-hackers
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> I'm not entirely sure how I should replace those DirectFunctionCall2 calls.

Basically what we want is for the called function to be able to use the
fcinfo->flinfo->fn_extra and fcinfo->flinfo->fn_mcxt fields of the
FmgrInfo struct that GIN has set up for the btree_gin support function.

The fast but somewhat scary way to do it would just be to pass through
the flinfo pointer as-is.  Imagine that fmgr.c grows a set of functions
like

Datum
DontKnowWhatToCallThisFunctionCall2(PGFunction func,                                   FmgrInfo *flinfo, Oid collation,
                                 Datum arg1, Datum arg2)
 
{   FunctionCallInfoData fcinfo;   Datum        result;
   InitFunctionCallInfoData(fcinfo, flinfo, 2, collation, NULL, NULL);
   fcinfo.arg[0] = arg1;   fcinfo.arg[1] = arg2;   fcinfo.argnull[0] = false;   fcinfo.argnull[1] = false;
   result = (*func) (&fcinfo);
   /* Check for null result, since caller is clearly not expecting one */   if (fcinfo.isnull)       elog(ERROR,
"function%p returned NULL", (void *) func);
 
   return result;
}

and then you'd just pass through the fcinfo->flinfo you got.

The reason this is kind of scary is that it's just blithely assuming
that the function won't look at the *other* fields of the FmgrInfo.
If it did, it would likely get very confused, since those fields
would be describing the GIN support function, not the function we're
calling.

We could alternatively have this trampoline function set up a fresh, local
FmgrInfo struct that it zeroes except for copying fn_extra and fn_mcxt
from the caller's struct, and then it copies fn_extra back again on the
way out.  That's a few more cycles but it would be safer, I think; if the
function tried to look at the other fields such as fn_oid it would see
obviously bogus data.

BTW, while I'm looking at this ... isn't gin_enum_cmp broken on its face?
It's using DirectFunctionCall2 to call enum_cmp, and that's wrong because
DirectFunctionCall2 does not pass through a flinfo but enum_cmp needs to
have one.  I've not tested, but I'm certain that this would dump core if
asked to compare odd-numbered enum OIDs.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Checksums by default?
Next
From: Andrew Dunstan
Date:
Subject: Re: [HACKERS] btree_gin and btree_gist for enums