diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2023-06-13 15:58:37 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2023-06-13 15:58:37 -0400 |
commit | cc6974df16e0dfb106c4caefd9d42d332a04247d (patch) | |
tree | 4dedc6caf27d59cd05c42483863f5a3238e77a2e | |
parent | bd590d1fea1ba9245c791d589eea94d2dbad5a2b (diff) | |
download | postgresql-cc6974df16e0dfb106c4caefd9d42d332a04247d.tar.gz postgresql-cc6974df16e0dfb106c4caefd9d42d332a04247d.zip |
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
-rw-r--r-- | src/backend/rewrite/rewriteHandler.c | 4 | ||||
-rw-r--r-- | src/test/regress/expected/updatable_views.out | 32 | ||||
-rw-r--r-- | src/test/regress/sql/updatable_views.sql | 17 |
3 files changed, 52 insertions, 1 deletions
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 5086b90df5f..cc00ee47776 100644 --- 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 8ecc58b1d0a..c27e2e803dd 100644 --- 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 7739b3be76d..eaee0b7e1d7 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -1424,6 +1424,23 @@ 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 -- |