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

tglsfdc · tglsfdc · commit ca08ea52bfbc · 2019-09-12T18:29:18.000-04:00
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 148e632, but not being aware of any user-visible consequences, he quite reasonably didn't back-patch. This patch is simply a back-patch of 148e632, 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
@@ -1198,7 +1198,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/updatable_views.out b/src/test/regress/expected/updatable_views.out
@@ -1604,31 +1604,31 @@ UPDATE rw_view1 SET a = a + 5; -- should fail
 ERROR:  new row violates WITH 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;

Original file line number	Diff line number	Diff line change
`@@ -1198,7 +1198,7 @@ ExecInitModifyTable(ModifyTable node, EState estate, int eflags)`
`1198`	`1198`	`{`
`1199`	`1199`	`WithCheckOption wco = (WithCheckOption ) lfirst(ll);`
`1200`	`1200`	`ExprState wcoExpr = ExecInitExpr((Expr ) wco->qual,`
`1201`		`- mtstate->mt_plans[i]);`
	`1201`	`+ &mtstate->ps);`
`1202`	`1202`
`1203`	`1203`	`wcoExprs = lappend(wcoExprs, wcoExpr);`
`1204`	`1204`	`}`