Re: On disable_cost - Mailing list pgsql-hackers

From Robert Haas
Subject Re: On disable_cost
Date
Msg-id CA+TgmobwgL1XyV4uyUd26Nxff5WVA7+9XUED4yjpvft83_MBAw@mail.gmail.com
Whole thread Raw
In response to Re: On disable_cost  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: On disable_cost  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Mon, May 6, 2024 at 9:39 AM Robert Haas <robertmhaas@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



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: backend stuck in DataFileExtend
Next
From: Justin Pryzby
Date:
Subject: Re: backend stuck in DataFileExtend