Date: Mon, 8 Jul 2024 17:20:11 -0700
From: Noah Misch <noah@leadboat.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
	Isaac Morland <isaac.morland@gmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: Faster "SET search_path"
Message-ID: <20240709002011.87.nmisch@google.com>
References: <20230802050720.GB866741@nathanxps13>
 <CAMsGm5dt6szoTe5oAzk5nfbHHOEpMDVhLTOGWCJGCQ3Wq8+J0w@mail.gmail.com>
 <fec7f3bea50112323d9b6e700b5e2762a14af809.camel@j-davis.com>
 <20230807223908.GB1284531@nathanxps13>
 <3b2d57ead15798adcbf6a53a15e35cdbadb2c147.camel@j-davis.com>
 <5bb4d9c21cc918e5d05021bf0d5e0e7d34a0377c.camel@j-davis.com>
 <abf4ce8804e0e05dff8c1725ae6a8ed28b7d66e0.camel@j-davis.com>
 <695668acc40b937258985669f3d55b94cce82c3c.camel@j-davis.com>
 <20240630223047.1f.nmisch@google.com>
 <cbb4ba82320a66c1e4a7c8fb3aa189ed9fd3908a.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <cbb4ba82320a66c1e4a7c8fb3aa189ed9fd3908a.camel@j-davis.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240709002011.87.nmisch%40google.com>
Precedence: bulk

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_auth_members, it seems like either
> AUTHMEMROLEMEM or AUTHMEMMEMROLE work, and to use both would be
> redundant. Am I missing something, or should I just pick one
> arbitrarily (or by some convention)?

One suffices.  initialize_acl() is the only other place that invals on this
catalog, so consider it a fine convention to mimic.

> >  and pg_database changes also need to invalidate this
> > cache.  (pg_database affects the ACL_CREATE_TEMP case in
> > pg_namespace_aclmask_ext()
> 
> I am having trouble finding an actual problem with ACL_CREATE_TEMP.
> search_path ACL checks are normally bypassed for the temp namespace, so
> it can only happen when the actual temp namespace name (e.g.
> pg_temp_NNN) is explicitly included. In that case, the mask is
> ACL_USAGE, so the two branches in pg_namespace_aclmask_ext() are
> equivalent, right?

I don't know.

> This question is not terribly important for the fix, because
> invalidating for each pg_database change is still necessary as you
> point out