Re: Patch: add GiST support for BOX @> POINT queries

2011/6/17 Andrew Tipton <andrew(dot)t(dot)tipton(at)gmail(dot)com>:
> On Fri, Jun 10, 2011 at 22:16, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> wrote:
>>
>> Isn't it worth adding new consistent function for those purposes? The
>> approach in the patch as stands looks kludge to me.
>
> Thanks for your review.  Coming back to this patch after a few months'
> time, I have to say it looks pretty hackish to my eyes as well. :)
>
> I've attempted to add a new consistent function,
> gist_boxpoint_consistent(), but the GiST subsystem doesn't call it --
> it continues to call gist_box_consistent().  My very simple testcase
> is:
>
> CREATE TABLE test (key TEXT PRIMARY KEY, boundary BOX NOT NULL);
> CREATE INDEX ON test USING gist (boundary);
> INSERT INTO test VALUES ('a', '(2,2,5,5)'), ('b', '(4,4,8,8)'), ('c',
> '(7,7,11,11)');
> SELECT * FROM test WHERE boundary @> '(4,4)'::POINT;
>
> Prior to my patch, this query is executed as a straightforward seqscan.
>
> Once I add a new strategy to pg_amop.h:
> + DATA(insert ( 2593   603 600 7 s  433 783 0 ));
>
> (603 is the BOX oid, 600 is the POINT oid, and 433 is the @> operator oid):
> ...the plan switches to an index scan and gist_box_consistent() is
> called;  at this point, the query fails to return the correct results.
>
> But even after adding the new consistent proc to pg_proc.h:
> + DATA(insert OID = 8000 (  gist_boxpoint_consistent    PGNSP PGUID 12
> 1 0 0 f f f t f i 5 0 16 "2281 600 23 26 2281" _null_ _null_ _null_
> _null_   gist_boxpoint_consistent _null_ _null_ _null_ ));
>
> And adding it as a new support function in pg_amproc.h:
> + DATA(insert ( 2593   603 600 1 8000 ));
> + DATA(insert ( 2593   603 600 2 2583 ));
> + DATA(insert ( 2593   603 600 3 2579 ));
> + DATA(insert ( 2593   603 600 4 2580 ));
> + DATA(insert ( 2593   603 600 5 2581 ));
> + DATA(insert ( 2593   603 600 6 2582 ));
> + DATA(insert ( 2593   603 600 7 2584 ));
>
> ...my gist_boxpoint_consistent() function still doesn't get called.
>
> At this point I'm a bit lost -- while pg_amop.h has plenty of examples
> of crosstype comparison operators for btree index methods, there are
> none for GiST.  Is GiST somehow a special case in this regard?

It was I that was lost. As Tom mentioned, GiST indexes have records in
pg_amop in their specialized way. I found gist_point_consistent has
some kind of hack for that and pg_amop for point_ops records have
multiple crosstype for that. So, if I understand correctly your first
approach modifying gist_box_consistent was the right way, although
trivial issues should be fixed. Also, you may want to follow point_ops
when you are worried if the counterpart operator of commutator should
be registered or not.

Looking around those mechanisms, it occurred to me that you mentioned
only box @> point. Why don't you add circly @> point, poly @> point as
well as box? Is that hard?

Regards,

--
Hitoshi Harada