On 2018/10/26 18:59, Tom Lane wrote: > Amit Langote <Langote_Amit_f8@xxxxxxxxxxxxx> writes: >> On 2018/10/26 18:16, Tom Lane wrote: >>> 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. > > After a quick look around, I think that making systable_begin/endscan > do this is a nonstarter; there are just too many call sites that would > be affected. Now, you could imagine specifying that indexes on system > catalogs (in practice, only btree) have to clean up at endscan time > but other index types don't, so that only operations that might be > scanning user indexes need to have suitable wrapping contexts. Not sure > there's a lot of benefit to that, though. By "core code", I didn't mean systable_being/endscan, but rather check_exclusion_or_unique_constraint() or its core-side caller(s). But maybe I misunderstood something about your diagnosis upthread where you said: "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," Thanks, Amit
