2011/2/24 Andrew Tipton <andrew.t.tip...@gmail.com>: > While playing around with the BOX and POINT datatypes, I was surprised to > note that BOX @> POINT (and likewise POINT <@ BOX) queries were not using > the GiST index I had created on the BOX column. The attached patch adds a > new strategy @>(BOX,POINT) to the box_ops opclass. Internally, > gist_box_consistent simply transforms the POINT into its corresponding BOX. > This is my first Postgres patch, and I wasn't able to figure out how to go > about creating a regression test for this change. (All existing tests do > pass, but none of them seem to specifically test index behaviour.)
I reviewed the patch and worried about hard-wired magic number as StrategyNumber. At least you should use #define to indicate the number's meaning. In addition, the modified gist_box_consistent() is too dangerous; q_box is declared in the if block locally and is referenced, which pointer is passed to the outer process of the block. AFAIK if the local memory of each block is alive outside if block is platform-dependent. Isn't it worth adding new consistent function for those purposes? The approach in the patch as stands looks kludge to me. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
