Fix usage of whole-row variables in WCO and RLS policy expressions.

Since WITH CHECK OPTION was introduced, ExecInitModifyTable has
initialized WCO expressions with the wrong plan node as parent -- that is,
it passed its input subplan not the ModifyTable node itself.  Up to now
we thought this was harmless, but bug #16006 from Vinay Banakar shows it's
not: if the input node is a SubqueryScan then ExecInitWholeRowVar can get
confused into doing the wrong thing.  (The fact that ExecInitWholeRowVar
contains such logic is certainly a horrid kluge that doesn't deserve to
live, but figuring out another way to do that is a task for some other day.)

Andres had already noticed the wrong-parent mistake and fixed it in commit
148e632c0, but not being aware of any user-visible consequences, he quite
reasonably didn't back-patch.  This patch is simply a back-patch of
148e632c0, plus addition of a test case based on bug #16006.  I also added
the test case to v12/HEAD, even though the bug is already fixed there.

Back-patch to all supported branches.  9.4 lacks RLS policies so the
new test case doesn't work there, but I'm pretty sure a test could be
devised based on using a whole-row Var in a plain WITH CHECK OPTION
condition.  (I lack the cycles to do so myself, though.)

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/16006-99290d2e4642cbd5@postgresql.org
Discussion: https://postgr.es/m/20181205225213.hiwa3kgoxeybqcqv@alap3.anarazel.de

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index c6193473481122f54eefd631d9a192470edc8d35..91198924a0d93a1a2448aa56ebffd4d12364e6e5 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1671,7 +1671,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                {
                        WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
                        ExprState  *wcoExpr = ExecInitExpr((Expr *) wco->qual,
-                                                                                          mtstate->mt_plans[i]);
+                                                                                          &mtstate->ps);
 
                        wcoExprs = lappend(wcoExprs, wcoExpr);
                }

diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 22a43260ea8f58d664ec227dec772d01410d32c7..f82f93ea414cd3d74f7d61d0a9e84ead70b2852f 100644 (file)
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -3511,6 +3511,42 @@ DROP OPERATOR <<< (int, int);
 DROP FUNCTION op_leak(int, int);
 RESET SESSION AUTHORIZATION;
 DROP TABLE rls_tbl;
+-- Bug #16006: whole-row Vars in a policy don't play nice with sub-selects
+SET SESSION AUTHORIZATION regress_rls_alice;
+CREATE TABLE rls_tbl (a int, b int, c int);
+CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1));
+ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
+ALTER TABLE rls_tbl FORCE ROW LEVEL SECURITY;
+INSERT INTO rls_tbl SELECT 10, 20, 30;
+EXPLAIN (VERBOSE, COSTS OFF)
+INSERT INTO rls_tbl
+  SELECT * FROM (SELECT b, c FROM rls_tbl ORDER BY a) ss;
+                                QUERY PLAN                                
+--------------------------------------------------------------------------
+ Insert on regress_rls_schema.rls_tbl
+   ->  Subquery Scan on ss
+         Output: ss.b, ss.c, NULL::integer
+         ->  Sort
+               Output: rls_tbl_1.b, rls_tbl_1.c, rls_tbl_1.a
+               Sort Key: rls_tbl_1.a
+               ->  Subquery Scan on rls_tbl_1
+                     Output: rls_tbl_1.b, rls_tbl_1.c, rls_tbl_1.a
+                     ->  Seq Scan on regress_rls_schema.rls_tbl rls_tbl_2
+                           Output: rls_tbl_2.b, rls_tbl_2.c, rls_tbl_2.a
+                           Filter: (rls_tbl_2.* >= ROW(1, 1, 1))
+(11 rows)
+
+INSERT INTO rls_tbl
+  SELECT * FROM (SELECT b, c FROM rls_tbl ORDER BY a) ss;
+SELECT * FROM rls_tbl;
+ a  | b  | c  
+----+----+----
+ 10 | 20 | 30
+ 20 | 30 |   
+(2 rows)
+
+DROP TABLE rls_tbl;
+RESET SESSION AUTHORIZATION;
 --
 -- Clean up objects
 --

diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 2e080521dba36720f13bb02f5532e5ab964f6436..9bc0b4024ed2c8af99cf6b3755598b354120e96a 100644 (file)
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -1665,31 +1665,31 @@ UPDATE rw_view1 SET a = a + 5; -- should fail
 ERROR:  new row violates check option for view "rw_view1"
 DETAIL:  Failing row contains (15).
 EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (5);
-                          QUERY PLAN                           
----------------------------------------------------------------
+                       QUERY PLAN                        
+---------------------------------------------------------
  Insert on base_tbl b
    ->  Result
-         SubPlan 1
-           ->  Index Only Scan using ref_tbl_pkey on ref_tbl r
-                 Index Cond: (a = b.a)
-         SubPlan 2
-           ->  Seq Scan on ref_tbl r_1
+   SubPlan 1
+     ->  Index Only Scan using ref_tbl_pkey on ref_tbl r
+           Index Cond: (a = b.a)
+   SubPlan 2
+     ->  Seq Scan on ref_tbl r_1
 (7 rows)
 
 EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5;
-                           QUERY PLAN                            
------------------------------------------------------------------
+                        QUERY PLAN                         
+-----------------------------------------------------------
  Update on base_tbl b
    ->  Hash Semi Join
          Hash Cond: (b.a = r.a)
          ->  Seq Scan on base_tbl b
          ->  Hash
                ->  Seq Scan on ref_tbl r
-         SubPlan 1
-           ->  Index Only Scan using ref_tbl_pkey on ref_tbl r_1
-                 Index Cond: (a = b.a)
-         SubPlan 2
-           ->  Seq Scan on ref_tbl r_2
+   SubPlan 1
+     ->  Index Only Scan using ref_tbl_pkey on ref_tbl r_1
+           Index Cond: (a = b.a)
+   SubPlan 2
+     ->  Seq Scan on ref_tbl r_2
 (11 rows)
 
 DROP TABLE base_tbl, ref_tbl CASCADE;

diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index a23b3cb05479db2aa8113f34bff98135305436fc..54bf205476351df91d47117547282e5db309175a 100644 (file)
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1621,6 +1621,25 @@ DROP FUNCTION op_leak(int, int);
 RESET SESSION AUTHORIZATION;
 DROP TABLE rls_tbl;
 
+-- Bug #16006: whole-row Vars in a policy don't play nice with sub-selects
+SET SESSION AUTHORIZATION regress_rls_alice;
+CREATE TABLE rls_tbl (a int, b int, c int);
+CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1));
+
+ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
+ALTER TABLE rls_tbl FORCE ROW LEVEL SECURITY;
+
+INSERT INTO rls_tbl SELECT 10, 20, 30;
+EXPLAIN (VERBOSE, COSTS OFF)
+INSERT INTO rls_tbl
+  SELECT * FROM (SELECT b, c FROM rls_tbl ORDER BY a) ss;
+INSERT INTO rls_tbl
+  SELECT * FROM (SELECT b, c FROM rls_tbl ORDER BY a) ss;
+SELECT * FROM rls_tbl;
+
+DROP TABLE rls_tbl;
+RESET SESSION AUTHORIZATION;
+
 --
 -- Clean up objects
 --