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

Commit69f526a

Browse files
committed
Mark read/write expanded values as read-only in ExecProject().
If a plan node output expression returns an "expanded" datum, and thatoutput column is referenced in more than one place in upper-level plannodes, we need to ensure that what is returned is a read-only referencenot a read/write reference. Otherwise one of the referencing sites couldscribble on or even delete the expanded datum before we have evaluated theothers. Commit1dc5ebc, which introduced this feature, supposedthat it'd be sufficient to make SubqueryScan nodes force their outputcolumns to read-only state. The folly of that was revealed by bug #14174from Andrew Gierth, and really should have been immediately obviousconsidering that the planner will happily optimize SubqueryScan nodesout of the plan without any regard for this issue.The safest fix seems to be to make ExecProject() force its results intoread-only state; that will cover every case where a plan node returnsexpression results. Actually we can delegate this to ExecTargetList()since we can recursively assume that plain Vars will not referenceread-write datums. That should keep the extra overhead down to somethingminimal. We no longer need ExecMakeSlotContentsReadOnly(), which wasintroduced only in support of the idea that just a few plan node typeswould need to do this.In the future it would be nice to have the planner account for this problemand inject force-to-read-only expression evaluation nodes into only theplaces where there's a risk of multiple evaluation. That's not a suitablesolution for 9.5 or even 9.6 at this point, though.Report: <20160603124628.9932.41279@wrigleys.postgresql.org>
1 parent04ae11f commit69f526a

File tree

6 files changed

+89
-58
lines changed

6 files changed

+89
-58
lines changed

‎src/backend/executor/execQual.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5350,15 +5350,24 @@ ExecCleanTargetListLength(List *targetlist)
53505350
* of *isDone = ExprMultipleResult signifies a set element, and a return
53515351
* of *isDone = ExprEndResult signifies end of the set of tuple.
53525352
* We assume that *isDone has been initialized to ExprSingleResult by caller.
5353+
*
5354+
* Since fields of the result tuple might be multiply referenced in higher
5355+
* plan nodes, we have to force any read/write expanded values to read-only
5356+
* status. It's a bit annoying to have to do that for every projected
5357+
* expression; in the future, consider teaching the planner to detect
5358+
* actually-multiply-referenced Vars and insert an expression node that
5359+
* would do that only where really required.
53535360
*/
53545361
staticbool
53555362
ExecTargetList(List*targetlist,
5363+
TupleDesctupdesc,
53565364
ExprContext*econtext,
53575365
Datum*values,
53585366
bool*isnull,
53595367
ExprDoneCond*itemIsDone,
53605368
ExprDoneCond*isDone)
53615369
{
5370+
Form_pg_attribute*att=tupdesc->attrs;
53625371
MemoryContextoldContext;
53635372
ListCell*tl;
53645373
boolhaveDoneSets;
@@ -5384,6 +5393,10 @@ ExecTargetList(List *targetlist,
53845393
&isnull[resind],
53855394
&itemIsDone[resind]);
53865395

5396+
values[resind]=MakeExpandedObjectReadOnly(values[resind],
5397+
isnull[resind],
5398+
att[resind]->attlen);
5399+
53875400
if (itemIsDone[resind]!=ExprSingleResult)
53885401
{
53895402
/* We have a set-valued expression in the tlist */
@@ -5437,6 +5450,10 @@ ExecTargetList(List *targetlist,
54375450
&isnull[resind],
54385451
&itemIsDone[resind]);
54395452

5453+
values[resind]=MakeExpandedObjectReadOnly(values[resind],
5454+
isnull[resind],
5455+
att[resind]->attlen);
5456+
54405457
if (itemIsDone[resind]==ExprEndResult)
54415458
{
54425459
/*
@@ -5470,6 +5487,7 @@ ExecTargetList(List *targetlist,
54705487
econtext,
54715488
&isnull[resind],
54725489
&itemIsDone[resind]);
5490+
/* no need for MakeExpandedObjectReadOnly */
54735491
}
54745492
}
54755493

@@ -5595,6 +5613,7 @@ ExecProject(ProjectionInfo *projInfo, ExprDoneCond *isDone)
55955613
if (projInfo->pi_targetlist)
55965614
{
55975615
if (!ExecTargetList(projInfo->pi_targetlist,
5616+
slot->tts_tupleDescriptor,
55985617
econtext,
55995618
slot->tts_values,
56005619
slot->tts_isnull,

‎src/backend/executor/execTuples.c

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@
8888
#include"nodes/nodeFuncs.h"
8989
#include"storage/bufmgr.h"
9090
#include"utils/builtins.h"
91-
#include"utils/expandeddatum.h"
9291
#include"utils/lsyscache.h"
9392
#include"utils/typcache.h"
9493

@@ -813,52 +812,6 @@ ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
813812
returnExecStoreTuple(newTuple,dstslot,InvalidBuffer, true);
814813
}
815814

816-
/* --------------------------------
817-
*ExecMakeSlotContentsReadOnly
818-
*Mark any R/W expanded datums in the slot as read-only.
819-
*
820-
* This is needed when a slot that might contain R/W datum references is to be
821-
* used as input for general expression evaluation. Since the expression(s)
822-
* might contain more than one Var referencing the same R/W datum, we could
823-
* get wrong answers if functions acting on those Vars thought they could
824-
* modify the expanded value in-place.
825-
*
826-
* For notational reasons, we return the same slot passed in.
827-
* --------------------------------
828-
*/
829-
TupleTableSlot*
830-
ExecMakeSlotContentsReadOnly(TupleTableSlot*slot)
831-
{
832-
/*
833-
* sanity checks
834-
*/
835-
Assert(slot!=NULL);
836-
Assert(slot->tts_tupleDescriptor!=NULL);
837-
Assert(!slot->tts_isempty);
838-
839-
/*
840-
* If the slot contains a physical tuple, it can't contain any expanded
841-
* datums, because we flatten those when making a physical tuple. This
842-
* might change later; but for now, we need do nothing unless the slot is
843-
* virtual.
844-
*/
845-
if (slot->tts_tuple==NULL)
846-
{
847-
Form_pg_attribute*att=slot->tts_tupleDescriptor->attrs;
848-
intattnum;
849-
850-
for (attnum=0;attnum<slot->tts_nvalid;attnum++)
851-
{
852-
slot->tts_values[attnum]=
853-
MakeExpandedObjectReadOnly(slot->tts_values[attnum],
854-
slot->tts_isnull[attnum],
855-
att[attnum]->attlen);
856-
}
857-
}
858-
859-
returnslot;
860-
}
861-
862815

863816
/* ----------------------------------------------------------------
864817
*convenience initialization routines

‎src/backend/executor/nodeSubqueryscan.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,7 @@ SubqueryNext(SubqueryScanState *node)
5656
* We just return the subplan's result slot, rather than expending extra
5757
* cycles for ExecCopySlot(). (Our own ScanTupleSlot is used only for
5858
* EvalPlanQual rechecks.)
59-
*
60-
* We do need to mark the slot contents read-only to prevent interference
61-
* between different functions reading the same datum from the slot. It's
62-
* a bit hokey to do this to the subplan's slot, but should be safe
63-
* enough.
6459
*/
65-
if (!TupIsNull(slot))
66-
slot=ExecMakeSlotContentsReadOnly(slot);
67-
6860
returnslot;
6961
}
7062

‎src/include/executor/tuptable.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ extern Datum ExecFetchSlotTupleDatum(TupleTableSlot *slot);
163163
externHeapTupleExecMaterializeSlot(TupleTableSlot*slot);
164164
externTupleTableSlot*ExecCopySlot(TupleTableSlot*dstslot,
165165
TupleTableSlot*srcslot);
166-
externTupleTableSlot*ExecMakeSlotContentsReadOnly(TupleTableSlot*slot);
167166

168167
/* in access/common/heaptuple.c */
169168
externDatumslot_getattr(TupleTableSlot*slot,intattnum,bool*isnull);

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5368,7 +5368,45 @@ ERROR: value for domain orderedarray violates check constraint "sorted"
53685368
CONTEXT: PL/pgSQL function testoa(integer,integer,integer) line 5 at assignment
53695369
drop function arrayassign1();
53705370
drop function testoa(x1 int, x2 int, x3 int);
5371-
-- access to call stack
5371+
--
5372+
-- Test handling of expanded arrays
5373+
--
5374+
create function returns_rw_array(int) returns int[]
5375+
language plpgsql as $$
5376+
declare r int[];
5377+
begin r := array[$1, $1]; return r; end;
5378+
$$ stable;
5379+
create function consumes_rw_array(int[]) returns int
5380+
language plpgsql as $$
5381+
begin return $1[1]; end;
5382+
$$ stable;
5383+
-- bug #14174
5384+
explain (verbose, costs off)
5385+
select i, a from
5386+
(select returns_rw_array(1) as a offset 0) ss,
5387+
lateral consumes_rw_array(a) i;
5388+
QUERY PLAN
5389+
-----------------------------------------------------------------
5390+
Nested Loop
5391+
Output: i.i, (returns_rw_array(1))
5392+
-> Result
5393+
Output: returns_rw_array(1)
5394+
-> Function Scan on public.consumes_rw_array i
5395+
Output: i.i
5396+
Function Call: consumes_rw_array((returns_rw_array(1)))
5397+
(7 rows)
5398+
5399+
select i, a from
5400+
(select returns_rw_array(1) as a offset 0) ss,
5401+
lateral consumes_rw_array(a) i;
5402+
i | a
5403+
---+-------
5404+
1 | {1,1}
5405+
(1 row)
5406+
5407+
--
5408+
-- Test access to call stack
5409+
--
53725410
create function inner_func(int)
53735411
returns int as $$
53745412
declare _context text;

‎src/test/regress/sql/plpgsql.sql

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4237,7 +4237,37 @@ select testoa(1,2,1); -- fail at update
42374237
drop function arrayassign1();
42384238
drop function testoa(x1 int, x2 int, x3 int);
42394239
4240-
-- access to call stack
4240+
4241+
--
4242+
-- Test handling of expanded arrays
4243+
--
4244+
4245+
create function returns_rw_array(int) returns int[]
4246+
language plpgsql as $$
4247+
declare r int[];
4248+
begin r := array[$1, $1]; return r; end;
4249+
$$ stable;
4250+
4251+
create function consumes_rw_array(int[]) returns int
4252+
language plpgsql as $$
4253+
begin return $1[1]; end;
4254+
$$ stable;
4255+
4256+
-- bug #14174
4257+
explain (verbose, costs off)
4258+
select i, a from
4259+
(select returns_rw_array(1) as a offset 0) ss,
4260+
lateral consumes_rw_array(a) i;
4261+
4262+
select i, a from
4263+
(select returns_rw_array(1) as a offset 0) ss,
4264+
lateral consumes_rw_array(a) i;
4265+
4266+
4267+
--
4268+
-- Test access to call stack
4269+
--
4270+
42414271
create function inner_func(int)
42424272
returns int as $$
42434273
declare _context text;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp