Re: Review: B-Tree emulation for GIN

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Jeff Davis <pgsql(at)j-davis(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review: B-Tree emulation for GIN
Date: 2009-03-05 15:26:48
Message-ID: 49AFEF38.3040904@sigaev.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> The GIN_EXTRACT_VALUE macro returns a pointer to a static 'entries'
> variable. That doesn't seem safe. Is it really never possible to have to
> two GIN searches in a plan, both calling and using the value returned
> by extractValue simultaneously? In any case that seems like a pretty
> weak assumption.
Fixed.

> You might want to declare extra_data as just "void *", instead of an
> array of pointers. The data type implementation might want to store
> something there that's not per-key, but applies to the whole query. I
> see that you're passing it to comparePartial, but that seems to be just
> future-proofing. What kind of a data type are you envisioning that would

wildspeed module (http://www.sigaev.ru/cvsweb/cvsweb.cgi/wildspeed/) - for each
key from it's needed to store some info. Right now it's coded directly in Datum,
but it looks ugly (at least for me).

It's possible to clarify interface by introducing new type:

typedef void* OpaquePtr;

Then, prototypes will be:
extractQuery(..., OpaquePtr* extra_data[])
consistent(...., OpaquePtr extra_data[])
comparePartial(..., OpaquePtr extra_data)

Or another option: partial match feature is new for 8.4, so we could change
interface:
typedef struct KeyData {
bool pmatch,
void *extra_data;
} KeyData;

Datum *extractQuery(Datum query, int32 *nkeys, StrategyNumber n, KeyData* data[])
bool consistent(bool check[], StrategyNumber n, Datum query, bool *recheck,
KeyData data[])
comparePartial(Datum partial_key, Datum key, KeyData *data);

> make use of it? It seems that you could pass the same information in the
> partial key Datum itself that extractQuery returns. You're currently
> using it as a way to avoid some palloc's in gin_tsquery_consistent().
> That seems like a pretty dirty hack. I doubt there's any meaningful
> performance advantage from that, but if there is, I think you could use
> a statically allocated array instead.

It's easy to un-dirty that hack, but before I'd like to see your comments about
thoughts above.

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

Attachment Content-Type Size
btree_gin-0.11.gz application/x-tar 14.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matteo Beccati 2009-03-05 15:47:30 Re: [HACKERS] Re: BUG #4689: Expanding the length of a VARCHAR column should not induce a table rewrite
Previous Message Tom Lane 2009-03-05 14:47:55 Re: Make SIGHUP less painful if pg_hba.conf is not readable