Re: WIP: SP-GiST, Space-Partitioned GiST

Teodor Sigaev <teodor(at)sigaev(dot)ru> writes:
> [ spgist patch ]

I've been working through this patch and fixing assorted things.
There are a couple of issues that require some discussion:

1. It took me awhile to realize it, but there are actually three
different datatypes that can be stored in an SPGist index: the prefix
value of an inner tuple, the key value for each node (downlink) in an
inner tuple, and the data value of a leaf tuple. One reason this was
not obvious is that only two of these types can be configured by the
opclass config function; the leaf tuple datatype is hard-wired to be
the same as the indexed column's type. Why is that? It seems to me
to be both confusing and restrictive. For instance, if you'd designed
the suffix tree opclass just a little differently, it would be wanting
to store "char" not text in leaf nodes. Shouldn't we change this to
allow the leaf data type to be specified explicitly?

2. The code is a bit self-contradictory about whether the index is
complete or not. The top-level ambuild and aminsert code rejects
NULLs (but there seems to be some provision for storing nulls in
leaf tuples --- I'm confused what that's used for, but apparently
not for actual data nulls). If this is a designed, permanent limitation
then SPGist indexes can never be expected to represent every heap tuple,
which means there is no value in full-index scans. However there is
code in spgWalk() to support a full-index scan. It's dead code, because
pg_am.amoptionalkey is not set for spgist and so the planner will never
create a qual-free index scan plan. Perhaps we should rip out the
full-index-scan code? Or do you have plans to support indexing nulls
later?

regards, tom lane