Here are my first comments. I haven't read the actual index implementation, yet.
I think traversal value is a useful addition. It is nice that the implementation for the range types is also changed. My questions about them are: Do reconstructedValues is required now? Wouldn't it be cleaner to use the new field on the prefix tree implementation, too? We haven't had specific memory context for reconstructedValues. Why is it required for the new field? > + Memory for <structfield>traversalValues</> should be allocated in > + the default context, but make sure to switch to > + <structfield>traversalMemoryContext</> before allocating memory > + for pointers themselves. This sentence is not understandable. Shouldn't it say "the memory should *not* be allocated in the default context"? What are the "pointers themselves"? The box opclass is broken because of the floating point arithmetics of the box type. You can reproduce it with the attached script. We really need to fix the geometric types, before building more on them. They fail to serve the only purpose they are useful for, demonstrating features. It think the opclass should support the cross data type operators like box @> point. Ideally we should do it by using multiple opclasses in an opfamily. The floating problem will bite us again in here, because some of the operators are not using FP macros. The tests check not supported operator "@". It should be "<@". It is nice that the opclass doesn't support long deprecated operators. We needs tests for the remaining operators and for adding new values to the index. I am not sure create_index.sql is the best place for them. Maybe we should use the test scripts of "box". > + #define LT -1 > + #define GT 1 > + #define EQ 0 Using these numbers is a very well established pattern. I don't think macros make the code any more readable. > + /* SP-GiST API functions */ > + Datum spg_box_quad_config(PG_FUNCTION_ARGS); > + Datum spg_box_quad_choose(PG_FUNCTION_ARGS); > + Datum spg_box_quad_picksplit(PG_FUNCTION_ARGS); > + Datum spg_box_quad_inner_consistent(PG_FUNCTION_ARGS); > + Datum spg_box_quad_leaf_consistent(PG_FUNCTION_ARGS); I guess they should go to the header file.
box-index-test.sql
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
