Re: btree_gin and btree_gist for enums

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: btree_gin and btree_gist for enums
Date: 2017-02-23 21:41:41
Message-ID: 8258.1487886101@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2017-02-23 21:45:00 Re: btree_gin and btree_gist for enums
Previous Message Bruce Momjian 2017-02-23 21:37:45 Re: Checksums by default?