Re: Faster "SET search_path"

2024-07-08 Thread Noah Misch
On Mon, Jul 08, 2024 at 04:39:21PM -0700, Jeff Davis wrote: > On Sun, 2024-06-30 at 15:30 -0700, Noah Misch wrote: > > You're caching the result of object_aclcheck(NamespaceRelationId, > > ...), so > > pg_auth_members changes > > Thank you for the report. > > Question: to check for changes to pg_

Re: Faster "SET search_path"

2024-07-08 Thread Jeff Davis
On Sun, 2024-06-30 at 15:30 -0700, Noah Misch wrote: > You're caching the result of object_aclcheck(NamespaceRelationId, > ...), so > pg_auth_members changes Thank you for the report. Question: to check for changes to pg_auth_members, it seems like either AUTHMEMROLEMEM or AUTHMEMMEMROLE work, an

Re: Faster "SET search_path"

2024-06-30 Thread Noah Misch
On Tue, Nov 14, 2023 at 08:13:25PM -0800, Jeff Davis wrote: > On Thu, 2023-10-19 at 19:01 -0700, Jeff Davis wrote: > > 0003: Cache for recomputeNamespacePath. > > Committed Commit f26c236 wrote: > Add cache for recomputeNamespacePath(). > > When search_path is changed to something th

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 -- I

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 a

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 othe

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 before

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. > > > > Attac

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 at

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 M

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 p

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 trying

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, s

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 invocation

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 slower

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 proced

Re: Faster "SET search_path"

2023-08-02 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 c

Re: Faster "SET search_path"

2023-08-01 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 makin

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 ca

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 t

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 be

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 feel

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 be

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 ED

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 Transf

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