From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Bruno Wolff III <bruno(at)wolff(dot)to>, pgsql-general(at)lists(dot)postgresql(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Should pg 11 use a lot more memory building an spgist index? |
Date: | 2018-10-26 10:08:44 |
Message-ID: | b173372a-6c03-b5c3-08fc-64e8b4c3a817@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-hackers |
On 2018/10/26 18:59, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> 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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2018-10-26 10:20:13 | Re: Should pg 11 use a lot more memory building an spgist index? |
Previous Message | Tom Lane | 2018-10-26 09:59:43 | Re: Should pg 11 use a lot more memory building an spgist index? |
From | Date | Subject | |
---|---|---|---|
Next Message | Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?= | 2018-10-26 10:15:19 | Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER |
Previous Message | Tom Lane | 2018-10-26 10:06:41 | Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER |