Re: Faster "SET search_path"

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Nathan Bossart <nathandbossart(at)gmail(dot)com>, Isaac Morland <isaac(dot)morland(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Faster "SET search_path"
Date: 2024-07-08 23:39:21
Message-ID: cbb4ba82320a66c1e4a7c8fb3aa189ed9fd3908a.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)?

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

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

> and affects ROLE_PG_DATABASE_OWNER membership.)

Another good catch, thank you.

Patch attached. Contains a bit of cleanup and is intended for 17+18.

Regards,
Jeff Davis

Attachment Content-Type Size
v1-0001-Fix-missing-invalidations-for-search_path-cache.patch text/x-patch 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2024-07-08 23:51:23 Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
Previous Message Tom Lane 2024-07-08 23:05:32 Re: Why do we define HAVE_GSSAPI_EXT_H?