Re: Should pg 11 use a lot more memory building an spgist index?

On 2018/10/26 18:16, Tom Lane wrote:
> The core of the problem I see is that check_exclusion_or_unique_constraint
> does index_beginscan/index_rescan/index_endscan in a long-lived context,
> while spgendscan seems to have employed dice while deciding which of
> spgbeginscan's allocations it would bother to pfree.  This is ancient,
> though the specific case you have here can only be tested back to v10
> because the inet SPGIST opclass wasn't there before.
> 
> A quick review of the other index AM endscan methods seems to indicate
> that they all try to clean up their mess.  So maybe we should just make
> spgendscan do likewise.  Alternatively, we could decide that requiring
> endscan methods to free storage is not very safe, in which case it would
> be incumbent on check_exclusion_or_unique_constraint to make a temporary
> context to run the scan in.  But if we're going to do the latter, I think
> we oughta go full in and remove the retail pfree's from all the *endscan
> functions.  We'd also have to review other callers of
> index_beginscan/index_endscan to see which ones might also need their own
> temp contexts.  So that would surely end up being more invasive than
> just adding some pfree's to spgendscan would be.  Maybe in the long run
> it'd be worth it, but probably not in the short run, or for back-patching.

FWIW, I would prefer the latter.  Not that people write new AMs on a
regular basis because we gave them an easier interface via CREATE ACCESS
METHOD, but it still seems better for the core code to deal with memory
allocation/freeing to avoid running into issues like this.

Thanks,
Amit