Date: Mon, 17 Feb 2025 15:42:44 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: revamp row-security tracking
Message-ID: <Z7OtVDMEs_EKpu6d@nathan>
References: <Zz91RagtQg2s9497@nathan>
 <CAEZATCUDuzvQ8THEHNRRkg7Nia7OzZBA5PzwCm_c70M82pCnPg@mail.gmail.com>
 <Z03hjW3U9yERkDdG@nathan>
 <Z7N3fIDbIGIzGByR@nathan>
 <853593.1739815709@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <853593.1739815709@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/Z7OtVDMEs_EKpu6d%40nathan>
Precedence: bulk

On Mon, Feb 17, 2025 at 01:08:29PM -0500, Tom Lane wrote:
> I think you're being too impatient.  It's still an interesting
> topic, it just needs more thought to get to something committable.

Maybe I am.  Thanks for chiming in.

> I find this has-row-security marking problem to be comparable
> to the has-sublinks marking problem.  We've had tons of
> bugs-of-omission with that too, and the present code feels
> ugly and not any less prone to omissions than it ever was.
> 
> I wonder whether considering both problems together would yield any
> insights, following Polya's dictum that "the more general problem may
> be easier to solve".
> 
> One straightforward idea is to just not do the marking at all,
> but rather require places that want to know these properties
> to do a fresh search of the query tree when they want to know
> it.  That obviously has performance questions to answer, but
> it's easier to give answers to performance questions than
> "is this correct" questions.

That could be worth a try.  The reason I started with the global queue idea
was that we seem to reliably discover relations with RLS enabled, we just
tend to miss propagating that information to the top-level query.  We could
invent a separate query tree walker for discovering RLS, etc. to keep it
centralized.  However, besides the performance questions, it would be
another separate piece of code to keep updated.  Perhaps another variation
on this idea is to create a query walker that just looks for hasRowSecurity
flags throughout the tree (and to use that to mark the plan cache entry
appropriately).

-- 
nathan