Skip to content

Commit 50c1374

Browse files
tglsfdctglsfdc
committed
Fix permission tests for views/tables proven empty by constraint exclusion.
A view defined as "select <something> where false" had the curious property that the system wouldn't check whether users had the privileges necessary to select from it. More generally, permissions checks could be skipped for tables referenced in sub-selects or views that were proven empty by constraint exclusion (although some quick testing suggests this seldom happens in cases of practical interest). This happened because the planner failed to include rangetable entries for such tables in the finished plan. This was noticed in connection with erroneous handling of materialized views, but actually the issue is quite unrelated to matviews. Therefore, revert commit 200ba16 in favor of a more direct test for the real problem. Back-patch to 9.2 where the bug was introduced (by commit 7741dd6).
1 parent 200ba16 commit 50c1374

File tree

6 files changed

+52
-34
lines changed

6 files changed

+52
-34
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1159,7 +1159,9 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
11591159
/*
11601160
* It's possible that constraint exclusion proved the subquery empty. If
11611161
* so, it's convenient to turn it back into a dummy path so that we will
1162-
* recognize appropriate optimizations at this level.
1162+
* recognize appropriate optimizations at this query level. (But see
1163+
* create_append_plan in createplan.c, which has to reverse this
1164+
* substitution.)
11631165
*/
11641166
if (is_dummy_plan(rel->subplan))
11651167
{

‎src/backend/optimizer/plan/createplan.c

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -678,37 +678,63 @@ static Plan *
678678
create_append_plan(PlannerInfo *root, AppendPath *best_path)
679679
{
680680
Append *plan;
681-
List *tlist = build_relation_tlist(best_path->path.parent);
681+
RelOptInfo *rel = best_path->path.parent;
682+
List *tlist = build_relation_tlist(rel);
682683
List *subplans = NIL;
683684
ListCell *subpaths;
684685

685686
/*
686-
* It is possible for the subplans list to contain only one entry, or even
687-
* no entries. Handle these cases specially.
687+
* The subpaths list could be empty, if every child was proven empty by
688+
* constraint exclusion. In that case generate a dummy plan that returns
689+
* no rows.
688690
*
689-
* XXX ideally, if there's just one entry, we'd not bother to generate an
690-
* Append node but just return the single child. At the moment this does
691-
* not work because the varno of the child scan plan won't match the
692-
* parent-rel Vars it'll be asked to emit.
691+
* Note that an AppendPath with no members is also generated in certain
692+
* cases where there was no appending construct at all, but we know the
693+
* relation is empty (see set_dummy_rel_pathlist).
693694
*/
694695
if (best_path->subpaths == NIL)
695696
{
696-
/* Generate a Result plan with constant-FALSE gating qual */
697-
return (Plan *) make_result(root,
698-
tlist,
699-
(Node *) list_make1(makeBoolConst(false,
700-
false)),
701-
NULL);
697+
/*
698+
* If this is a dummy path for a subquery, we have to wrap the
699+
* subquery's original plan in a SubqueryScan so that setrefs.c will
700+
* do the right things. (In particular, it must pull up the
701+
* subquery's rangetable so that the executor will apply permissions
702+
* checks to those rels at runtime.)
703+
*/
704+
if (rel->rtekind == RTE_SUBQUERY)
705+
{
706+
Assert(is_dummy_plan(rel->subplan));
707+
return (Plan *) make_subqueryscan(tlist,
708+
NIL,
709+
rel->relid,
710+
rel->subplan);
711+
}
712+
else
713+
{
714+
/* Generate a Result plan with constant-FALSE gating qual */
715+
return (Plan *) make_result(root,
716+
tlist,
717+
(Node *) list_make1(makeBoolConst(false,
718+
false)),
719+
NULL);
720+
}
702721
}
703722

704-
/* Normal case with multiple subpaths */
723+
/* Build the plan for each child */
705724
foreach(subpaths, best_path->subpaths)
706725
{
707726
Path *subpath = (Path *) lfirst(subpaths);
708727

709728
subplans = lappend(subplans, create_plan_recurse(root, subpath));
710729
}
711730

731+
/*
732+
* XXX ideally, if there's just one child, we'd not bother to generate an
733+
* Append node but just return the single child. At the moment this does
734+
* not work because the varno of the child scan plan won't match the
735+
* parent-rel Vars it'll be asked to emit.
736+
*/
737+
712738
plan = make_append(subplans, tlist);
713739

714740
return (Plan *) plan;

‎src/test/regress/expected/matview.out

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -418,15 +418,3 @@ NOTICE: drop cascades to 3 other objects
418418
DETAIL: drop cascades to view v_test2
419419
drop cascades to materialized view mv_test2
420420
drop cascades to materialized view mv_test3
421-
-- test a corner case for "with no data" versus a query which yields no rows
422-
CREATE MATERIALIZED VIEW matview_unit_false AS SELECT false WHERE false WITH NO DATA;
423-
SELECT * FROM matview_unit_false;
424-
ERROR: materialized view "matview_unit_false" has not been populated
425-
HINT: Use the REFRESH MATERIALIZED VIEW command.
426-
REFRESH MATERIALIZED VIEW matview_unit_false;
427-
SELECT * FROM matview_unit_false;
428-
bool
429-
------
430-
(0 rows)
431-
432-
DROP MATERIALIZED VIEW matview_unit_false;

‎src/test/regress/expected/privileges.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ CREATE VIEW atestv1 AS SELECT * FROM atest1; -- ok
198198
/* The next *should* fail, but it's not implemented that way yet. */
199199
CREATE VIEW atestv2 AS SELECT * FROM atest2;
200200
CREATE VIEW atestv3 AS SELECT * FROM atest3; -- ok
201+
/* Empty view is a corner case that failed in 9.2. */
202+
CREATE VIEW atestv0 AS SELECT 0 as x WHERE false; -- ok
201203
SELECT * FROM atestv1; -- ok
202204
a | b
203205
---+-----
@@ -224,6 +226,8 @@ SELECT * FROM atestv3; -- ok
224226
-----+-----+-------
225227
(0 rows)
226228

229+
SELECT * FROM atestv0; -- fail
230+
ERROR: permission denied for relation atestv0
227231
CREATE VIEW atestv4 AS SELECT * FROM atestv3; -- nested view
228232
SELECT * FROM atestv4; -- ok
229233
one | two | three
@@ -1386,6 +1390,7 @@ drop table dep_priv_test;
13861390
drop sequence x_seq;
13871391
DROP FUNCTION testfunc2(int);
13881392
DROP FUNCTION testfunc4(boolean);
1393+
DROP VIEW atestv0;
13891394
DROP VIEW atestv1;
13901395
DROP VIEW atestv2;
13911396
-- this should cascade to drop atestv4

‎src/test/regress/sql/matview.sql

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,3 @@ CREATE MATERIALIZED VIEW mv_test3 AS SELECT * FROM mv_test2 WHERE moo = 12345;
129129
SELECT pg_relation_is_scannable('mv_test3'::regclass);
130130

131131
DROP VIEW v_test1 CASCADE;
132-
133-
-- test a corner case for "with no data" versus a query which yields no rows
134-
CREATE MATERIALIZED VIEW matview_unit_false AS SELECT false WHERE false WITH NO DATA;
135-
SELECT * FROM matview_unit_false;
136-
REFRESH MATERIALIZED VIEW matview_unit_false;
137-
SELECT * FROM matview_unit_false;
138-
DROP MATERIALIZED VIEW matview_unit_false;

‎src/test/regress/sql/privileges.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ CREATE VIEW atestv1 AS SELECT * FROM atest1; -- ok
147147
/* The next *should* fail, but it's not implemented that way yet. */
148148
CREATE VIEW atestv2 AS SELECT * FROM atest2;
149149
CREATE VIEW atestv3 AS SELECT * FROM atest3; -- ok
150+
/* Empty view is a corner case that failed in 9.2. */
151+
CREATE VIEW atestv0 AS SELECT 0 as x WHERE false; -- ok
150152

151153
SELECT * FROM atestv1; -- ok
152154
SELECT * FROM atestv2; -- fail
@@ -158,6 +160,7 @@ SET SESSION AUTHORIZATION regressuser4;
158160
SELECT * FROM atestv1; -- ok
159161
SELECT * FROM atestv2; -- fail
160162
SELECT * FROM atestv3; -- ok
163+
SELECT * FROM atestv0; -- fail
161164

162165
CREATE VIEW atestv4 AS SELECT * FROM atestv3; -- nested view
163166
SELECT * FROM atestv4; -- ok
@@ -828,6 +831,7 @@ drop sequence x_seq;
828831
DROP FUNCTION testfunc2(int);
829832
DROP FUNCTION testfunc4(boolean);
830833

834+
DROP VIEW atestv0;
831835
DROP VIEW atestv1;
832836
DROP VIEW atestv2;
833837
-- this should cascade to drop atestv4

0 commit comments

Comments
 (0)