Re: GiST: memory allocation, cleanup

Neil Conway <neilc@samurai.com> writes:
> Attached is a patch that makes some cleanups and improvements to GiST,
> as well as a few semi-related cleanups. Changes:
> ...

> QUESTION: given a ScanKey for an index scan, GiST overwrites the
> ScanKey's sk_func to point to the user-defined Consistent method
> (gistrescan() in gistscan.c). When doing a GiST index scan we can ensure
> that the sk_func is invoked in a short-lived memory context
> (gistfindnext() in gistget.c). Is it safe to assume that anywhere else
> in the backend that invokes the ScanKey's sk_func, it is done in a
> short-lived memory context?

I think you can assume that noplace else in the backend will invoke the
sk_func, period.  If it did, it'd be passing the wrong parameters ---
the Consistent method doesn't take the same args as a plain indexable
operator would.

> - mark the array that indicates NULLs that is passed to
> index_formtuple() as 'const', and fix the resulting fallout

I'm a bit dubious about this, mainly because you did not likewise
const-ify the other input arguments; it seems confusing to do a partial
const-ification.

> - after doing an index build, there was previously a long comment and a
> few lines of code that used the # of index tuples and heap tuples
> (computed by the index build) to update the optimizer's statistics. This
> code was duplicated 4 times (once for each type of index); I added a
> function called IndexUpdateStats() in catalog/index.c and changed each
> index's build function to call that function.

The only thing I don't like about this is that it's not apparent that
the function would close the heap & index relations, so the calling code
will now look like it's leaking the relation references.  Maybe call it
IndexCloseAndUpdateStats or something like that?

            regards, tom lane