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

Commita102f98

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 parentec56223 commita102f98

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
@@ -5333,15 +5333,24 @@ ExecCleanTargetListLength(List *targetlist)
53335333
* of *isDone = ExprMultipleResult signifies a set element, and a return
53345334
* of *isDone = ExprEndResult signifies end of the set of tuple.
53355335
* We assume that *isDone has been initialized to ExprSingleResult by caller.
5336+
*
5337+
* Since fields of the result tuple might be multiply referenced in higher
5338+
* plan nodes, we have to force any read/write expanded values to read-only
5339+
* status. It's a bit annoying to have to do that for every projected
5340+
* expression; in the future, consider teaching the planner to detect
5341+
* actually-multiply-referenced Vars and insert an expression node that
5342+
* would do that only where really required.
53365343
*/
53375344
staticbool
53385345
ExecTargetList(List*targetlist,
5346+
TupleDesctupdesc,
53395347
ExprContext*econtext,
53405348
Datum*values,
53415349
bool*isnull,
53425350
ExprDoneCond*itemIsDone,
53435351
ExprDoneCond*isDone)
53445352
{
5353+
Form_pg_attribute*att=tupdesc->attrs;
53455354
MemoryContextoldContext;
53465355
ListCell*tl;
53475356
boolhaveDoneSets;
@@ -5367,6 +5376,10 @@ ExecTargetList(List *targetlist,
53675376
&isnull[resind],
53685377
&itemIsDone[resind]);
53695378

5379+
values[resind]=MakeExpandedObjectReadOnly(values[resind],
5380+
isnull[resind],
5381+
att[resind]->attlen);
5382+
53705383
if (itemIsDone[resind]!=ExprSingleResult)
53715384
{
53725385
/* We have a set-valued expression in the tlist */
@@ -5420,6 +5433,10 @@ ExecTargetList(List *targetlist,
54205433
&isnull[resind],
54215434
&itemIsDone[resind]);
54225435

5436+
values[resind]=MakeExpandedObjectReadOnly(values[resind],
5437+
isnull[resind],
5438+
att[resind]->attlen);
5439+
54235440
if (itemIsDone[resind]==ExprEndResult)
54245441
{
54255442
/*
@@ -5453,6 +5470,7 @@ ExecTargetList(List *targetlist,
54535470
econtext,
54545471
&isnull[resind],
54555472
&itemIsDone[resind]);
5473+
/* no need for MakeExpandedObjectReadOnly */
54565474
}
54575475
}
54585476

@@ -5578,6 +5596,7 @@ ExecProject(ProjectionInfo *projInfo, ExprDoneCond *isDone)
55785596
if (projInfo->pi_targetlist)
55795597
{
55805598
if (!ExecTargetList(projInfo->pi_targetlist,
5599+
slot->tts_tupleDescriptor,
55815600
econtext,
55825601
slot->tts_values,
55835602
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
@@ -5304,7 +5304,45 @@ ERROR: value for domain orderedarray violates check constraint "sorted"
53045304
CONTEXT: PL/pgSQL function testoa(integer,integer,integer) line 5 at assignment
53055305
drop function arrayassign1();
53065306
drop function testoa(x1 int, x2 int, x3 int);
5307-
-- access to call stack
5307+
--
5308+
-- Test handling of expanded arrays
5309+
--
5310+
create function returns_rw_array(int) returns int[]
5311+
language plpgsql as $$
5312+
declare r int[];
5313+
begin r := array[$1, $1]; return r; end;
5314+
$$ stable;
5315+
create function consumes_rw_array(int[]) returns int
5316+
language plpgsql as $$
5317+
begin return $1[1]; end;
5318+
$$ stable;
5319+
-- bug #14174
5320+
explain (verbose, costs off)
5321+
select i, a from
5322+
(select returns_rw_array(1) as a offset 0) ss,
5323+
lateral consumes_rw_array(a) i;
5324+
QUERY PLAN
5325+
-----------------------------------------------------------------
5326+
Nested Loop
5327+
Output: i.i, (returns_rw_array(1))
5328+
-> Result
5329+
Output: returns_rw_array(1)
5330+
-> Function Scan on public.consumes_rw_array i
5331+
Output: i.i
5332+
Function Call: consumes_rw_array((returns_rw_array(1)))
5333+
(7 rows)
5334+
5335+
select i, a from
5336+
(select returns_rw_array(1) as a offset 0) ss,
5337+
lateral consumes_rw_array(a) i;
5338+
i | a
5339+
---+-------
5340+
1 | {1,1}
5341+
(1 row)
5342+
5343+
--
5344+
-- Test access to call stack
5345+
--
53085346
create function inner_func(int)
53095347
returns int as $$
53105348
declare _context text;

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4199,7 +4199,37 @@ select testoa(1,2,1); -- fail at update
41994199
drop function arrayassign1();
42004200
drop function testoa(x1 int, x2 int, x3 int);
42014201
4202-
-- access to call stack
4202+
4203+
--
4204+
-- Test handling of expanded arrays
4205+
--
4206+
4207+
create function returns_rw_array(int) returns int[]
4208+
language plpgsql as $$
4209+
declare r int[];
4210+
begin r := array[$1, $1]; return r; end;
4211+
$$ stable;
4212+
4213+
create function consumes_rw_array(int[]) returns int
4214+
language plpgsql as $$
4215+
begin return $1[1]; end;
4216+
$$ stable;
4217+
4218+
-- bug #14174
4219+
explain (verbose, costs off)
4220+
select i, a from
4221+
(select returns_rw_array(1) as a offset 0) ss,
4222+
lateral consumes_rw_array(a) i;
4223+
4224+
select i, a from
4225+
(select returns_rw_array(1) as a offset 0) ss,
4226+
lateral consumes_rw_array(a) i;
4227+
4228+
4229+
--
4230+
-- Test access to call stack
4231+
--
4232+
42034233
create function inner_func(int)
42044234
returns int as $$
42054235
declare _context text;

0 commit comments

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