Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit15b0421

Browse files
committed
Fix permission tests for views/tables proven empty by constraint exclusion.
A view defined as "select <something> where false" had the curious propertythat the system wouldn't check whether users had the privileges necessaryto select from it. More generally, permissions checks could be skippedfor tables referenced in sub-selects or views that were proven empty byconstraint exclusion (although some quick testing suggests this seldomhappens in cases of practical interest). This happened because the plannerfailed to include rangetable entries for such tables in the finished plan.This was noticed in connection with erroneous handling of materializedviews, but actually the issue is quite unrelated to matviews. Therefore,revert commit200ba16 in favor of a moredirect test for the real problem.Back-patch to 9.2 where the bug was introduced (by commit7741dd6).
1 parent4aed94f commit15b0421

File tree

4 files changed

+52
-15
lines changed

4 files changed

+52
-15
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,9 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
11291129
/*
11301130
* It's possible that constraint exclusion proved the subquery empty. If
11311131
* so, it's convenient to turn it back into a dummy path so that we will
1132-
* recognize appropriate optimizations at this level.
1132+
* recognize appropriate optimizations at this query level. (But see
1133+
* create_append_plan in createplan.c, which has to reverse this
1134+
* substitution.)
11331135
*/
11341136
if (is_dummy_plan(rel->subplan))
11351137
{

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

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -664,37 +664,63 @@ static Plan *
664664
create_append_plan(PlannerInfo*root,AppendPath*best_path)
665665
{
666666
Append*plan;
667-
List*tlist=build_relation_tlist(best_path->path.parent);
667+
RelOptInfo*rel=best_path->path.parent;
668+
List*tlist=build_relation_tlist(rel);
668669
List*subplans=NIL;
669670
ListCell*subpaths;
670671

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

690-
/*Normal case with multiple subpaths */
709+
/*Build the plan for each child */
691710
foreach(subpaths,best_path->subpaths)
692711
{
693712
Path*subpath= (Path*)lfirst(subpaths);
694713

695714
subplans=lappend(subplans,create_plan_recurse(root,subpath));
696715
}
697716

717+
/*
718+
* XXX ideally, if there's just one child, we'd not bother to generate an
719+
* Append node but just return the single child. At the moment this does
720+
* not work because the varno of the child scan plan won't match the
721+
* parent-rel Vars it'll be asked to emit.
722+
*/
723+
698724
plan=make_append(subplans,tlist);
699725

700726
return (Plan*)plan;

‎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/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
CREATEVIEWatestv2ASSELECT*FROM atest2;
149149
CREATEVIEWatestv3ASSELECT*FROM atest3;-- ok
150+
/* Empty view is a corner case that failed in 9.2.*/
151+
CREATEVIEWatestv0ASSELECT0as xWHERE 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
CREATEVIEWatestv4ASSELECT*FROM atestv3;-- nested view
163166
SELECT*FROM atestv4;-- ok
@@ -828,6 +831,7 @@ drop sequence x_seq;
828831
DROPFUNCTION testfunc2(int);
829832
DROPFUNCTION testfunc4(boolean);
830833

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

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp