Thread: BUG #13988: "plan should not reference subplan's variable" whilst using row level security

The following bug has been logged on the website:

Bug reference:      13988
Logged by:          Adam Guthrie
Email address:      asguthrie@gmail.com
PostgreSQL version: 9.5.1
Operating system:   OS X 10.11
Description:

The following sql:

****

CREATE TABLE a (
    id      INTEGER PRIMARY KEY
);

CREATE TABLE b (
    id      INTEGER PRIMARY KEY,
    a_id    INTEGER,
    text    TEXT
);

CREATE POLICY a_select ON b FOR SELECT
    USING ( EXISTS(SELECT FROM a WHERE a.id = b.a_id) );
ALTER TABLE b ENABLE ROW LEVEL SECURITY;

INSERT INTO a (id) VALUES (1);

INSERT INTO b (id, a_id, text) VALUES (1, 1, 'one');

CREATE ROLE test;
GRANT ALL ON ALL TABLES IN SCHEMA public TO test;
SET ROLE test;

SELECT * FROM b;

UPDATE b SET text = 'ONE' WHERE id = 1;

****

gives:

psql:/tmp/plan.sql:25: ERROR:  XX000: plan should not reference subplan's
variable
LOCATION:  finalize_plan, subselect.c:2624

whereas I believe it should give:

UPDATE 0

asguthrie@gmail.com wrote:
> The following bug has been logged on the website:
>
> Bug reference:      13988
> Logged by:          Adam Guthrie
> Email address:      asguthrie@gmail.com
> PostgreSQL version: 9.5.1
> Operating system:   OS X 10.11

For the benefit of those not in pgsql-general, this is already being
discussed here:
http://www.postgresql.org/message-id/CAC3DOVy2H7W5bGeVaJjq5XtKvxGNKiPkG_SjXNOqXYLB5ccFBA@mail.gmail.com

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 24 February 2016 at 21:42, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> asguthrie@gmail.com wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference:      13988
>> Logged by:          Adam Guthrie
>> Email address:      asguthrie@gmail.com
>> PostgreSQL version: 9.5.1
>> Operating system:   OS X 10.11
>
> For the benefit of those not in pgsql-general, this is already being
> discussed here:
> http://www.postgresql.org/message-id/CAC3DOVy2H7W5bGeVaJjq5XtKvxGNKiPkG_SjXNOqXYLB5ccFBA@mail.gmail.com
>

This can also be directly reproduced using updatable security barrier
views. The following is equivalent to what is happening with that RLS
setup:

CREATE TABLE a(id int);
CREATE TABLE b(id int, a_id int, text text);

CREATE VIEW v1 WITH (security_barrier=true) AS
  SELECT * FROM b WHERE false;
CREATE VIEW v2 WITH (security_barrier=true) AS
  SELECT * FROM v1 WHERE EXISTS (SELECT FROM a WHERE a.id = v1.a_id);

UPDATE v2 SET text = 'ONE' WHERE id = 1;


Debugging it, I have a theory as to the cause of the problem, which I
think is in security_barrier_replace_vars() --- when it finds a
matching Var that needs to be added to the targetlist that it is
building, it copies the existing Var and modifies it:

    /* New variable for subquery targetlist */
    newvar = copyObject(var);
    newvar->varno = newvar->varnoold = 1;
    ...

However, the Var found comes from a sublink subquery in the outer
query, and so has varlevelsup = 1, but newvar is for the new subquery
being built, so it needs to have varlevelsup set to 0, which that code
fails to do.

If that is indeed the problem, the fix is trivial, but I haven't had a
chance to test that theory yet -- hopefully I'll get some more time at
the weekend.

Regards,
Dean

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Debugging it, I have a theory as to the cause of the problem, which I
> think is in security_barrier_replace_vars() --- when it finds a
> matching Var that needs to be added to the targetlist that it is
> building, it copies the existing Var and modifies it:

>     /* New variable for subquery targetlist */
>     newvar = copyObject(var);
>     newvar->varno = newvar->varnoold = 1;
>     ...

> However, the Var found comes from a sublink subquery in the outer
> query, and so has varlevelsup = 1, but newvar is for the new subquery
> being built, so it needs to have varlevelsup set to 0, which that code
> fails to do.

Offhand, I'd think it more likely that this code should not be touching
outer-level vars at all?  There are few if any situations where it makes
sense to suppose that a Var of one level should be transformed into a Var
of a different level.

            regards, tom lane

On 26 February 2016 at 00:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> Debugging it, I have a theory as to the cause of the problem, which I
>> think is in security_barrier_replace_vars() --- when it finds a
>> matching Var that needs to be added to the targetlist that it is
>> building, it copies the existing Var and modifies it:
>
>>     /* New variable for subquery targetlist */
>>     newvar = copyObject(var);
>>     newvar->varno = newvar->varnoold = 1;
>>     ...
>
>> However, the Var found comes from a sublink subquery in the outer
>> query, and so has varlevelsup = 1, but newvar is for the new subquery
>> being built, so it needs to have varlevelsup set to 0, which that code
>> fails to do.
>

Sorry, part of that description was inaccurate. The Var found is in a
sublink subquery in the *current* query, not an outer query. This code
of course never examines outer queries.


> Offhand, I'd think it more likely that this code should not be touching
> outer-level vars at all?  There are few if any situations where it makes
> sense to suppose that a Var of one level should be transformed into a Var
> of a different level.
>

That's not really what it's doing. What's happenning here is that
expand_security_qual() is going to replace an RTE that has security
quals with a security barrier subquery RTE. So it searches the query
containing the RTE for any Vars that refer to it, in order to ensure
that the new RTE's subquery exposes the required columns, and the Vars
referring to that RTE are adjusted accordingly (not touching their
levels). In this case, the Var found is in the query's WHERE clause,
buried inside a sublink subquery, and so it has varlevelsup = 1. The
bug is simply that in building the *new* Var for the targetlist of the
new RTE subquery, it's copying the old Var but neglecting to set
varlevelsup to 0 on the new Var.

I just did a quick test, and setting newvar->varlevelsup to 0 does
indeed appear to fix the reported problem, and the SB views variant of
it.

Regards,
Dean

On 26 February 2016 at 08:27, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> I just did a quick test, and setting newvar->varlevelsup to 0 does
> indeed appear to fix the reported problem, and the SB views variant of
> it.
>

OK, confirmed. That was a pretty clear-cut bug in the updatable
security barrier views code.

Fix pushed to master and backpatched to 9.4.

Thanks for the report.

Regards,
Dean