Correctly update hasSubLinks while mutating a rule action.

rewriteRuleAction neglected to check for SubLink nodes in the
securityQuals of range table entries.  This could lead to failing
to convert such a SubLink to a SubPlan, resulting in assertion
crashes or weird errors later in planning.

In passing, fix some poor coding in rewriteTargetView:
we should not pass the source parsetree's hasSubLinks
field to ReplaceVarsFromTargetList's outer_hasSubLinks.
ReplaceVarsFromTargetList knows enough to ignore that
when a Query node is passed, but it's still confusing
and bad precedent: if we did try to update that flag
we'd be updating a stale copy of the parsetree.

Per bug #17972 from Alexander Lakhin.  This has been broken since
we added RangeTblEntry.securityQuals (although the presented test
case only fails back to 215b43cdc), so back-patch all the way.

Discussion: https://postgr.es/m/17972-f422c094237847d0@postgresql.org

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 5086b90df5f5a2a61616e7ecb36cadd0eef61f71..cc00ee47776294998fb37ec91e1a5121276b5a14 100644 (file)
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -477,6 +477,8 @@ rewriteRuleAction(Query *parsetree,
                    /* other RTE types don't contain bare expressions */
                    break;
            }
+           sub_action->hasSubLinks |=
+               checkExprHasSubLink((Node *) rte->securityQuals);
            if (sub_action->hasSubLinks)
                break;          /* no need to keep scanning rtable */
        }
@@ -3348,7 +3350,7 @@ rewriteTargetView(Query *parsetree, Relation view)
                                  view_targetlist,
                                  REPLACEVARS_REPORT_ERROR,
                                  0,
-                                 &parsetree->hasSubLinks);
+                                 NULL);
 
    /*
     * Update all other RTI references in the query that point to the view

diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 8ecc58b1d0add79db31bd5562f60fbc0a7dd3519..c27e2e803dd99e34f18786573d1ea3ad77b5d555 100644 (file)
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2838,6 +2838,38 @@ DROP VIEW v1;
 DROP TABLE t2;
 DROP TABLE t1;
 --
+-- Test sub-select in nested security barrier views, per bug #17972
+--
+CREATE TABLE t1 (a int);
+CREATE VIEW v1 WITH (security_barrier = true) AS
+  SELECT * FROM t1;
+CREATE RULE v1_upd_rule AS ON UPDATE TO v1 DO INSTEAD
+  UPDATE t1 SET a = NEW.a WHERE a = OLD.a;
+CREATE VIEW v2 WITH (security_barrier = true) AS
+  SELECT * FROM v1 WHERE EXISTS (SELECT 1);
+EXPLAIN (COSTS OFF) UPDATE v2 SET a = 1;
+                    QUERY PLAN                     
+---------------------------------------------------
+ Update on t1
+   InitPlan 1 (returns $0)
+     ->  Result
+   ->  Merge Join
+         Merge Cond: (t1.a = v1.a)
+         ->  Sort
+               Sort Key: t1.a
+               ->  Seq Scan on t1
+         ->  Sort
+               Sort Key: v1.a
+               ->  Subquery Scan on v1
+                     ->  Result
+                           One-Time Filter: $0
+                           ->  Seq Scan on t1 t1_1
+(14 rows)
+
+DROP VIEW v2;
+DROP VIEW v1;
+DROP TABLE t1;
+--
 -- Test CREATE OR REPLACE VIEW turning a non-updatable view into an
 -- auto-updatable view and adding check options in a single step
 --

diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index 7739b3be76d72034ce34a8256e8cc78c903faacf..eaee0b7e1d7164be271e5c7032211a5a0d2062c0 100644 (file)
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1423,6 +1423,23 @@ DROP VIEW v1;
 DROP TABLE t2;
 DROP TABLE t1;
 
+--
+-- Test sub-select in nested security barrier views, per bug #17972
+--
+CREATE TABLE t1 (a int);
+CREATE VIEW v1 WITH (security_barrier = true) AS
+  SELECT * FROM t1;
+CREATE RULE v1_upd_rule AS ON UPDATE TO v1 DO INSTEAD
+  UPDATE t1 SET a = NEW.a WHERE a = OLD.a;
+CREATE VIEW v2 WITH (security_barrier = true) AS
+  SELECT * FROM v1 WHERE EXISTS (SELECT 1);
+
+EXPLAIN (COSTS OFF) UPDATE v2 SET a = 1;
+
+DROP VIEW v2;
+DROP VIEW v1;
+DROP TABLE t1;
+
 --
 -- Test CREATE OR REPLACE VIEW turning a non-updatable view into an
 -- auto-updatable view and adding check options in a single step