Re: Faster "SET search_path"

2023-12-05 Thread Jeff Davis
On Mon, 2023-11-20 at 17:13 -0800, Jeff Davis wrote: > Will commit 0005 soon. Committed. > I also attached a trivial 0006 patch that uses SH_STORE_HASH. I > wasn't > able to show much benefit, though, even when there's a bucket > collision. Perhaps there just aren't enough elements to matter --

Re: Faster "SET search_path"

2023-11-20 Thread Jeff Davis
On Thu, 2023-11-16 at 16:46 -0800, Jeff Davis wrote: > While I considered OOM during hash key initialization, I missed some > other potential out-of-memory hazards. Attached a fixup patch 0003, > which re-introduces one list copy but it simplifies things > substantially in addition to being safer

Re: Faster "SET search_path"

2023-11-16 Thread Jeff Davis
On Tue, 2023-11-14 at 20:13 -0800, Jeff Davis wrote: > On Thu, 2023-10-19 at 19:01 -0700, Jeff Davis wrote: > > 0003: Cache for recomputeNamespacePath. > > Committed with some further simplification around the OOM handling. While I considered OOM during hash key initialization, I missed some

Re: Faster "SET search_path"

2023-11-14 Thread Jeff Davis
On Thu, 2023-10-19 at 19:01 -0700, Jeff Davis wrote: > 0003: Cache for recomputeNamespacePath. Committed with some further simplification around the OOM handling. Instead of using MCXT_ALLOC_NO_OOM, it just temporarily sets the cache invalid while copying the string, and sets it valid again

Re: Faster "SET search_path"

2023-10-19 Thread Jeff Davis
On Fri, 2023-09-15 at 11:31 -0700, Jeff Davis wrote: > On Tue, 2023-09-12 at 13:55 -0700, Jeff Davis wrote: > > On Mon, 2023-08-07 at 15:39 -0700, Nathan Bossart wrote: > > > 0003 is looking pretty good, too, but I think we > > > should get some more eyes on it, given the complexity. > > > >

Re: Faster "SET search_path"

2023-09-15 Thread Jeff Davis
On Tue, 2023-09-12 at 13:55 -0700, Jeff Davis wrote: > On Mon, 2023-08-07 at 15:39 -0700, Nathan Bossart wrote: > > 0003 is looking pretty good, too, but I think we > > should get some more eyes on it, given the complexity. > > Attached rebased version of 0003. Is someone else planning to look

Re: Faster "SET search_path"

2023-09-12 Thread Jeff Davis
On Mon, 2023-08-07 at 15:39 -0700, Nathan Bossart wrote: > 0003 is looking pretty good, too, but I think we > should get some more eyes on it, given the complexity. Attached rebased version of 0003. -- Jeff Davis PostgreSQL Contributor Team - AWS From f5b055aea1bf08928de1bffcfd7b202e28847595

Re: Faster "SET search_path"

2023-08-22 Thread Jeff Davis
On Wed, 2023-08-16 at 15:09 -0700, Jeff Davis wrote: > To bring the overhead closer to zero we need to somehow avoid > repeating > so much work in guc.c, though. If we don't go around it, another > approach would be to separate GUC setting into two phases: one that > does the checks, and one that

Re: Faster "SET search_path"

2023-08-16 Thread Jeff Davis
On Tue, 2023-08-15 at 13:04 -0400, Robert Haas wrote: > I suspect that dodging the GUC stack machinery is not a very good > idea. The timing of when TRY/CATCH blocks are called is different > from > when subtransactions are aborted, and that distinction has messed me > up more than once when

Re: Faster "SET search_path"

2023-08-15 Thread Robert Haas
On Mon, Aug 7, 2023 at 7:24 PM Jeff Davis wrote: > I might just avoid guc.c entirely and directly set > namespace_search_path and baseSearchPathValid=false. The main thing we > lose there is the GUC stack (AtEOXact_GUC()), but there's already a > PG_TRY/PG_FINALLY block in fmgr_security_definer,

Re: Faster "SET search_path"

2023-08-07 Thread Jeff Davis
On Mon, 2023-08-07 at 15:39 -0700, Nathan Bossart wrote: > 0001 and 0002 LGTM. I'll probably go ahead with 0001 soon. Simple and effective. But for 0002, I was thinking about trying to optimize so check_search_path() only gets called once at the beginning, rather than for each function

Re: Faster "SET search_path"

2023-08-07 Thread Nathan Bossart
0001 and 0002 LGTM. 0003 is looking pretty good, too, but I think we should get some more eyes on it, given the complexity. On Mon, Aug 07, 2023 at 12:57:27PM -0700, Jeff Davis wrote: > (Aside: part of the reason set_config_option() is slow is because of > the lookup in guc_hashtab. That's

Re: Faster "SET search_path"

2023-08-07 Thread Jeff Davis
On Wed, 2023-08-02 at 01:14 -0400, Isaac Morland wrote: > > > On Sat, 2023-07-29 at 12:44 -0400, Isaac Morland wrote: > > > > Essentially, "just" observe efficiently (somehow) that no > > > > change is > > > > needed, and skip changing it? ... > Speaking as someone who uses a lot of stored

Re: Faster "SET search_path"

2023-08-03 Thread Jeff Davis
On Tue, 2023-08-01 at 21:52 -0700, Nathan Bossart wrote: > I > typically see this done with two ѕeparate lists and forboth(). Agreed, done. > > Any reason not to use hash_combine() here? Thank you, fixed. > > I changed it to move the hook so that it's called after retrieving > > from > > the

Re: Faster "SET search_path"

2023-08-02 Thread Jeff Davis
On Wed, 2023-08-02 at 01:14 -0400, Isaac Morland wrote: > I don’t think the fact that an optimization might suddenly not work > in a certain situation is a reason not to optimize. What would our > query planner look like if we took that approach? ... > Instead, we should try to find ways of

Re: Faster "SET search_path"

2023-08-01 Thread Jeff Davis
On Tue, 2023-08-01 at 22:07 -0700, Nathan Bossart wrote: > I wonder if this is a good enough reason to _not_ proceed with this > optimization.  At the moment, I'm on the fence about it. I was wondering the same thing. It's something that could reasonably be explained to users; it's not what I'd

Re: Faster "SET search_path"

2023-08-01 Thread Isaac Morland
On Wed, 2 Aug 2023 at 01:07, Nathan Bossart wrote: > On Mon, Jul 31, 2023 at 10:28:31PM -0700, Jeff Davis wrote: > > On Sat, 2023-07-29 at 12:44 -0400, Isaac Morland wrote: > >> Essentially, "just" observe efficiently (somehow) that no change is > >> needed, and skip changing it? > > > > I gave

Re: Faster "SET search_path"

2023-08-01 Thread Nathan Bossart
On Mon, Jul 31, 2023 at 10:28:31PM -0700, Jeff Davis wrote: > On Sat, 2023-07-29 at 12:44 -0400, Isaac Morland wrote: >> Essentially, "just" observe efficiently (somehow) that no change is >> needed, and skip changing it? > > I gave this a try and it speeds things up some more. > > There might

Re: Faster "SET search_path"

2023-08-01 Thread Nathan Bossart
On Tue, Aug 01, 2023 at 04:59:33PM -0700, Jeff Davis wrote: > + List*pair= lfirst(lc); > + char*name= linitial(pair); > + char*value = lsecond(pair); This is definitely a nitpick, but this List of Lists business

Re: Faster "SET search_path"

2023-08-01 Thread Jeff Davis
On Sat, 2023-07-29 at 21:51 -0700, Nathan Bossart wrote: > On Sat, Jul 29, 2023 at 08:59:01AM -0700, Jeff Davis wrote: > > 0001: Transform the settings in proconfig into a List for faster > > processing. This is simple and speeds up any proconfig setting. > > This looks straightforward.  It might

Re: Faster "SET search_path"

2023-07-31 Thread Jeff Davis
On Sat, 2023-07-29 at 12:44 -0400, Isaac Morland wrote: > Essentially, "just" observe efficiently (somehow) that no change is > needed, and skip changing it? I gave this a try and it speeds things up some more. There might be a surprise factor with an optimization like that, though. If someone

Re: Faster "SET search_path"

2023-07-31 Thread Robert Haas
On Sat, Jul 29, 2023 at 11:59 AM Jeff Davis wrote: > Unfortunately, adding a "SET search_path" clause to functions slows > them down. The attached patches close the performance gap > substantially. I haven't reviewed the code but I like the concept a lot. This is badly needed. -- Robert Haas

Re: Faster "SET search_path"

2023-07-29 Thread Nathan Bossart
On Sat, Jul 29, 2023 at 08:59:01AM -0700, Jeff Davis wrote: > 0001: Transform the settings in proconfig into a List for faster > processing. This is simple and speeds up any proconfig setting. This looks straightforward. It might be worth combining the common parts of ProcessGUCArray() and

Re: Faster "SET search_path"

2023-07-29 Thread Isaac Morland
On Sat, 29 Jul 2023 at 11:59, Jeff Davis wrote: Unfortunately, adding a "SET search_path" clause to functions slows > them down. The attached patches close the performance gap > substantially. > > Changes: > > 0001: Transform the settings in proconfig into a List for faster > processing. This is

Faster "SET search_path"

2023-07-29 Thread Jeff Davis
Improve performance of "SET search_path". Motivation: Creating functions with a "SET search_path" clause is safer and more secure because the function behavior doesn't change based on the caller's search_path setting. Setting search_path in the function declaration is especially important for