From ca08ea52bfbc0e02eb7e866bb05bf166f15c9b75 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 12 Sep 2019 18:29:18 -0400 Subject: [PATCH] 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 --- src/backend/executor/nodeModifyTable.c | 2 +- src/test/regress/expected/updatable_views.out | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 9fb2ea17775..b6d32c92911 100644 --- 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 index 1393dbc5c5c..dffdaa1ee03 100644 --- 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;