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
