Re: Review: B-Tree emulation for GIN - Mailing list pgsql-hackers

From Teodor Sigaev
Subject Re: Review: B-Tree emulation for GIN
Date
Msg-id 49AFEF38.3040904@sigaev.ru
Whole thread Raw
In response to Re: Review: B-Tree emulation for GIN  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
Responses Re: Review: B-Tree emulation for GIN  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
List 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@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Attachment

pgsql-hackers by date:

Previous
From: Joshua Tolley
Date:
Subject: Re: Make SIGHUP less painful if pg_hba.conf is not readable
Next
From: David Fetter
Date:
Subject: Re: Prepping to break every past release...