GiST: memory allocation, cleanup - Mailing list pgsql-patches

Attached is a patch that makes some cleanups and improvements to GiST,
as well as a few semi-related cleanups. Changes:

- previously, GiST did not make any use of the backend's memory context
infrastructure. This made implementing indexes using GiST unnecessarily
difficult and fragile: if your implementation of a GiST method leaked
memory, this could result in a potentially large memory leak in
PostgreSQL itself. This patch changes GiST so that all invocations of
used-defined GiST methods are done within a short-lived memory context.
In practice, this means GiST implementors need not use pfree, unless
they are doing a _lot_ of allocations in their method implementations.

As part of this work, memory contexts were also used to cleanup a lot of
ugly and difficult to maintain memory management code in gist.c itself.

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?

- previously, src/include/access/gist.h included both the "public" API
that is intended for use by GiST implementors as well as declarations
for the GiST implementation itself. I've created a new header file
"gist_private.h" and moved the latter declarations there.

- remove some "extern" keywords from function definitions in
contrib/btree_gist, and do some more minor code cleanup there

- minor GiST documentation cleanup

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

- 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.

- in GiST, move the definition of a bunch of scope-local variables to
the most tightly-enclosing scope. In other words, code like:

{
    int a, b;

    a = ...;
    if (a)
    {
        b = ...;
    }
}

was changed to move the definition of "b" into the inner scope. This is
per good style, IMHO (and it is consistent with the rest of the
PostgreSQL source).

- moved gistfreestack() to gistscan.c and made it "static", since it is
only used there

- cleaned up various other things in gist.c: added some comments,
basically tried to impose some sanity

Comments welcome. I would like to apply this to 8.1 at some point after
we branch for 8.0. I thought I would post this patch to get some
feedback before starting on further GiST work (speculatively, adding
support for concurrency and making it WAL-safe).

-Neil