From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Neil Conway <neilc(at)samurai(dot)com> |
Cc: | pgsql-patches <pgsql-patches(at)postgresql(dot)org>, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su> |
Subject: | Re: GiST: memory allocation, cleanup |
Date: | 2004-11-05 21:31:14 |
Message-ID: | 13069.1099690274@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Neil Conway <neilc(at)samurai(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2004-11-05 21:56:50 | Re: GiST: memory allocation, cleanup |
Previous Message | Tom Lane | 2004-11-05 20:37:54 | Re: [PATCHES] CVS should die |