Re: Clarification on Role Access Rights to Table Indexes

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Jeff Davis <pgsql(at)j-davis(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ayush Vatsa <ayushvatsa1810(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Clarification on Role Access Rights to Table Indexes
Date: 2025-10-09 21:18:03
Message-ID: aOgmi6avE6qMw_6t@nathan
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

On Thu, Oct 09, 2025 at 10:39:32AM -0500, Nathan Bossart wrote:
> On Wed, Oct 08, 2025 at 08:28:01PM -0700, Jeff Davis wrote:
>> Actually, now I'm unsure. v4-0001 is taking a lock on the table before
>> checking privileges, whereas v4-0002 is going to some effort to avoid
>> that. Is that because the latter is taking a ShareLock?
>
> I was confused by this, too. We seem to go to great lengths to avoid
> taking a lock before checking permissions in RangeVarGetRelidExtended(),
> but in pg_prewarm() and this stats code, we are taking the lock first.
> pg_prewarm() can't use RangeVarGetRelid because you give it the OID, but
> I'm not seeing why stat_utils.c can't use it. We should probably fix this.
> I wouldn't be surprised if there are other examples.

I spent some time trying to change pg_prewarm() to check permissions before
locking and came up with the attached. There are certainly issues with the
patch, but this at least demonstrates the complexity required. I'm tempted
to say that this is more trouble than it's worth, but it does feel a little
weird to leave it as-is.

There's a similar pattern in get_rel_from_relname() in dblink.c, which also
seems to only be used with an AccessShareLock (like pg_prewarm). My best
guess from reading lots of code, commit messages, and old e-mails in the
archives is that the original check-privileges-before-locking work was
never completed.

I'm currently leaning towards continuing with v4 of the patch set. 0001
and 0003 are a little weird in that a concurrent change could lead to a
"could not find parent table" ERROR, but IIUC that is an extremely remote
possibility.

--
nathan

Attachment Content-Type Size
0001-pg_prewarm-privilege-test.patch text/plain 6.3 KB

In response to

Browse pgsql-general by date

  From Date Subject
Next Message Vu Le (JData - HN) 2025-10-10 08:26:28 Upgrade & Rollback plan: My customer requests rollback via old-version standby (13 ↔ 17) — need community advice
Previous Message Rob Sargent 2025-10-09 20:54:56 Re: High latency and profiling

Browse pgsql-hackers by date

  From Date Subject
Next Message John H 2025-10-09 21:48:33 Re: Making pg_rewind faster
Previous Message Andres Freund 2025-10-09 21:16:49 Re: Buffer locking is special (hints, checksums, AIO writes)