Re: btree_gin and ranges

On 12/22/2014 03:15 AM, Michael Paquier wrote:
> On Thu, Dec 18, 2014 at 4:13 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> On 10/22/2014 01:55 PM, Teodor Sigaev wrote:
>>>
>>> Suggested patch adds GIN support contains operator for ranges over scalar
>>> column.
>>>
>>> It allows more effective GIN scan. Currently, queries like
>>> SELECT * FROM test_int4 WHERE i <= 1 and i >= 1
>>> will be excuted by GIN with two scans: one is from mines infinity to 1 and
>>> another is from -1 to plus infinity. That's because GIN is "generalized"
>>> and it
>>> doesn't know a semantics of operation.
>>>
>>> With patch it's possible to rewrite query with ranges
>>> SELECT * FROM test_int4 WHERE i <@ '[-1, 1]'::int4range
>>> and GIN index will support this query with single scan from -1 to 1.
>>>
>>> Patch provides index support only for existing range types: int4, int8,
>>> numeric,
>>> date and timestamp with and without time zone.
>>
>>
>> I started to look at this, but very quickly got carried away into
>> refactoring away the macros. Defining long functions as macros makes
>> debugging quite difficult, and it's not nice to read or edit the source code
>> either.
>>
>> I propose the attached refactoring (it doesn't include your range-patch yet,
>> just refactoring the existing code). It turns the bulk of the macros into
>> static functions. GIN_SUPPORT macro still defines the datatype-specific
>> functions, but they are now very thin wrappers that just call the
>> corresponding generic static functions.
>>
>> It's annoying that we need the concept of a leftmost value, for the < and <=
>> queries. Without that, we could have the support functions look up the
>> required datatype information from the type cache, based on the datatype of
>> the passed argument. Then you could easily use the btree_gin support
>> functions also for user-defined datatypes. But that needs bigger changes,
>> and this is a step in the right direction anyway.
> I just had a look at the refactoring patch and ISTM that this is a
> good step forward in terms of readability. Teodor, I am noticing that
> your patch cannot apply once the refactoring is done. Could you rebase
> your patch once the refactoring is pushed?s

I've pushed the refactoring patch. Here's a version of Teodor's patch,
rebased over the pushed patch.

Teodor's patch could use some more comments. The
STOP_SCAN/MATCH_SCAN/CONT_SCAN macros are a good idea, but they probably
should go into src/include/access/gin.h so that they can be used in all
compare_partial implementations.

- Heikki