On Mon, May 6, 2024 at 9:39 AM Robert Haas <robertmh...@gmail.com> wrote: > It's not very clear that this mechanism is actually 100% reliable,
It isn't. Here's a test case. As a non-superuser, do this: create table foo (a int, b text, primary key (a)); insert into foo values (1, 'Apple'); alter table foo enable row level security; alter table foo force row level security; create policy p1 on foo as permissive using (ctid in ('(0,1)', '(0,2)')); begin; declare c cursor for select * from foo; fetch from c; explain update foo set b = 'Manzana' where current of c; update foo set b = 'Manzana' where current of c; The explain produces this output: Update on foo (cost=10000000000.00..10000000008.02 rows=0 width=0) -> Tid Scan on foo (cost=10000000000.00..10000000008.02 rows=1 width=38) TID Cond: (ctid = ANY ('{"(0,1)","(0,2)"}'::tid[])) Filter: CURRENT OF c Unless I'm quite confused, the point of the code is to force CurrentOfExpr to be a TID Cond, and it normally succeeds in doing so, because WHERE CURRENT OF cursor_name has to be the one and only WHERE condition for a normal UPDATE. I tried various cases involving views and CTEs and got nowhere. But then I wrote a patch to make the regression tests fail if a baserel's restrictinfo list contains a CurrentOfExpr and also some other qual, and a couple of row-level security tests failed (and nothing else). Which then allowed me to construct the example above, where there are two possible TID quals and the logic in tidpath.c latches onto the wrong one. The actual UPDATE fails like this: ERROR: WHERE CURRENT OF is not supported for this table type ...because ExecEvalCurrentOfExpr supposes that the only way it can be reached is for an FDW without the necessary support, but actually in this case it's planner error that gets us here. Fortunately, there's no real reason for anyone to ever do something like this, or at least I can't see one, so the fact that it doesn't work probably doesn't really matter that much. And you can argue that the only problem here is that the costing hack just didn't get updated for RLS and now needs to be a bit more clever. But I think it'd be better to find a way of making it less hacky. With the way the code is structured right now, the chances of anyone understanding that RLS might have an impact on its correctness were just about nil, IMHO. -- Robert Haas EDB: http://www.enterprisedb.com