Re: Yet another fast GiST build

On 08/09/2020 21:33, Pavel Borisov wrote:
> > Thanks! Did you measure the quality of the built index somehow? The
> > ordering shouldn't make any difference to the build speed, but it
> > affects the shape of the resulting index and the speed of queries
> > against it.
>
> Again I've tried random select tests near axes and haven't noticed any
> performance difference between ordinary gist build and z-ordered one.
> The same is for selects far from axes. Theoretically, there may be a
> possible slowdown for particular points inside the MBR which crosses the
> axis but I haven't tried to dig so deep and haven't tested performance
> as a function of coordinate.
>
> So I feel this patch is not about select speed optimization.

Ok, thank for confirming.

I've been reviewing the patch today. The biggest changes I've made have
been in restructuring the code in gistbuild.c for readability, but there
are a bunch of smaller changes throughout. Attached is what I've got so
far, squashed into one patch. I'm continuing to review it, but a couple
of questions so far:

In the gistBuildCallback(), you're skipping the tuple if 'tupleIsAlive
== false'. That seems fishy, surely we need to index recently-dead
tuples, too. The normal index build path isn't skipping them either.

How does the 'sortsupport' routine interact with
'compress'/'decompress'? Which representation is passed to the
comparator routine: the original value from the table, the compressed
representation, or the decompressed representation? Do the
comparetup_index_btree() and readtup_index() routines agree with that?

- Heikki