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

Commiteb9d1f0

Browse files
committed
Repair more failures with SubPlans in multi-row VALUES lists.
Commit9b63c13 turns out to have been fundamentally misguided:the parent node's subPlan list is by no means the only way in whicha child SubPlan node can be hooked into the outer execution state.As shown in bug #16213 from Matt Jibson, we can also get short-livedtuple table slots added to the outer es_tupleTable list. At this pointI have little faith that there aren't other possible connections aswell; the long time it took to notice this problem shows that thisisn't a heavily-exercised situation.Therefore, revert that fix, returning to the coding that passed aNULL parent plan pointer down to the transiently-built subexpressions.That gives us a pretty good guarantee that they won't hook into theouter executor state in any way. But then we need some other solutionto make SubPlans work. Adopt the solution speculated about in theprevious commit's log message: do expression initialization at planstartup for just those VALUES rows containing SubPlans, abandoning thegoal of reclaiming memory intra-query for those rows. In practice itseems unlikely that queries containing a vast number of VALUES rowswould be using SubPlans in them, so this should not give up much.(BTW, this test case also refutes my claim in connection with the priorcommit that the issue only arises with use of LATERAL. That was justwrong: some variants of SubLink always produce SubPlans.)As with previous patch, back-patch to all supported branches.Discussion:https://postgr.es/m/16213-871ac3bc208ecf23@postgresql.org
1 parent20a1dc1 commiteb9d1f0

File tree

4 files changed

+101
-34
lines changed

4 files changed

+101
-34
lines changed

‎src/backend/executor/nodeValuesscan.c

Lines changed: 50 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include"executor/executor.h"
2727
#include"executor/nodeValuesscan.h"
28+
#include"optimizer/clauses.h"
2829

2930

3031
staticTupleTableSlot*ValuesNext(ValuesScanState*node);
@@ -48,7 +49,7 @@ ValuesNext(ValuesScanState *node)
4849
EState*estate;
4950
ExprContext*econtext;
5051
ScanDirectiondirection;
51-
List*exprlist;
52+
intcurr_idx;
5253

5354
/*
5455
* get information from the estate and scan state
@@ -65,19 +66,11 @@ ValuesNext(ValuesScanState *node)
6566
{
6667
if (node->curr_idx<node->array_len)
6768
node->curr_idx++;
68-
if (node->curr_idx<node->array_len)
69-
exprlist=node->exprlists[node->curr_idx];
70-
else
71-
exprlist=NIL;
7269
}
7370
else
7471
{
7572
if (node->curr_idx >=0)
7673
node->curr_idx--;
77-
if (node->curr_idx >=0)
78-
exprlist=node->exprlists[node->curr_idx];
79-
else
80-
exprlist=NIL;
8174
}
8275

8376
/*
@@ -88,11 +81,12 @@ ValuesNext(ValuesScanState *node)
8881
*/
8982
ExecClearTuple(slot);
9083

91-
if (exprlist)
84+
curr_idx=node->curr_idx;
85+
if (curr_idx >=0&&curr_idx<node->array_len)
9286
{
87+
List*exprlist=node->exprlists[curr_idx];
88+
List*exprstatelist=node->exprstatelists[curr_idx];
9389
MemoryContextoldContext;
94-
List*oldsubplans;
95-
List*exprstatelist;
9690
Datum*values;
9791
bool*isnull;
9892
ListCell*lc;
@@ -106,30 +100,28 @@ ValuesNext(ValuesScanState *node)
106100
ReScanExprContext(econtext);
107101

108102
/*
109-
* Build the expression eval state in the econtext's per-tuple memory.
110-
* This is a tad unusual, but we want to delete the eval state again
111-
* when we move to the next row, to avoid growth of memory
112-
* requirements over a long values list.
103+
* Do per-VALUES-row work in the per-tuple context.
113104
*/
114105
oldContext=MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
115106

116107
/*
117-
* The expressions might contain SubPlans (this is currently only
118-
* possible if there's a sub-select containing a LATERAL reference,
119-
* otherwise sub-selects in a VALUES list should be InitPlans). Those
120-
* subplans will want to hook themselves into our subPlan list, which
121-
* would result in a corrupted list after we delete the eval state. We
122-
* can work around this by saving and restoring the subPlan list.
123-
* (There's no need for the functionality that would be enabled by
124-
* having the list entries, since the SubPlans aren't going to be
125-
* re-executed anyway.)
108+
* Unless we already made the expression eval state for this row,
109+
* build it in the econtext's per-tuple memory. This is a tad
110+
* unusual, but we want to delete the eval state again when we move to
111+
* the next row, to avoid growth of memory requirements over a long
112+
* values list. For rows in which that won't work, we already built
113+
* the eval state at plan startup.
126114
*/
127-
oldsubplans=node->ss.ps.subPlan;
128-
node->ss.ps.subPlan=NIL;
129-
130-
exprstatelist= (List*)ExecInitExpr((Expr*)exprlist,&node->ss.ps);
131-
132-
node->ss.ps.subPlan=oldsubplans;
115+
if (exprstatelist==NIL)
116+
{
117+
/*
118+
* Pass parent as NULL, not my plan node, because we don't want
119+
* anything in this transient state linking into permanent state.
120+
* The only expression type that might wish to do so is a SubPlan,
121+
* and we already checked that there aren't any.
122+
*/
123+
exprstatelist= (List*)ExecInitExpr((Expr*)exprlist,NULL);
124+
}
133125

134126
/* parser should have checked all sublists are the same length */
135127
Assert(list_length(exprstatelist)==slot->tts_tupleDescriptor->natts);
@@ -261,13 +253,38 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags)
261253
scanstate->curr_idx=-1;
262254
scanstate->array_len=list_length(node->values_lists);
263255

264-
/* convert list of sublists into array of sublists for easy addressing */
256+
/*
257+
* Convert the list of expression sublists into an array for easier
258+
* addressing at runtime. Also, detect whether any sublists contain
259+
* SubPlans; for just those sublists, go ahead and do expression
260+
* initialization. (This avoids problems with SubPlans wanting to connect
261+
* themselves up to the outer plan tree. Notably, EXPLAIN won't see the
262+
* subplans otherwise; also we will have troubles with dangling pointers
263+
* and/or leaked resources if we try to handle SubPlans the same as
264+
* simpler expressions.)
265+
*/
265266
scanstate->exprlists= (List**)
266267
palloc(scanstate->array_len*sizeof(List*));
268+
scanstate->exprstatelists= (List**)
269+
palloc0(scanstate->array_len*sizeof(List*));
267270
i=0;
268271
foreach(vtl,node->values_lists)
269272
{
270-
scanstate->exprlists[i++]= (List*)lfirst(vtl);
273+
List*exprs=castNode(List,lfirst(vtl));
274+
275+
scanstate->exprlists[i]=exprs;
276+
277+
/*
278+
* We can avoid the cost of a contain_subplans() scan in the simple
279+
* case where there are no SubPlans anywhere.
280+
*/
281+
if (estate->es_subplanstates&&
282+
contain_subplans((Node*)exprs))
283+
{
284+
scanstate->exprstatelists[i]= (List*)
285+
ExecInitExpr((Expr*)exprs,&scanstate->ss.ps);
286+
}
287+
i++;
271288
}
272289

273290
scanstate->ss.ps.ps_TupFromTlist= false;

‎src/include/nodes/execnodes.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1432,7 +1432,8 @@ typedef struct FunctionScanState
14321432
*
14331433
*rowcontextper-expression-list context
14341434
*exprlistsarray of expression lists being evaluated
1435-
*array_lensize of array
1435+
*exprstatelistsarray of expression state lists, for SubPlans only
1436+
*array_lensize of above arrays
14361437
*curr_idxcurrent array index (0-based)
14371438
*marked_idxmarked position (for mark/restore)
14381439
*
@@ -1441,6 +1442,12 @@ typedef struct FunctionScanState
14411442
*rowcontext, in which to build the executor expression state for each
14421443
*Values sublist. Resetting this context lets us get rid of expression
14431444
*state for each row, avoiding major memory leakage over a long values list.
1445+
*However, that doesn't work for sublists containing SubPlans, because a
1446+
*SubPlan has to be connected up to the outer plan tree to work properly.
1447+
*Therefore, for only those sublists containing SubPlans, we do expression
1448+
*state construction at executor start, and store those pointers in
1449+
*exprstatelists[]. NULL entries in that array correspond to simple
1450+
*subexpressions that are handled as described above.
14441451
* ----------------
14451452
*/
14461453
typedefstructValuesScanState
@@ -1451,6 +1458,8 @@ typedef struct ValuesScanState
14511458
intarray_len;
14521459
intcurr_idx;
14531460
intmarked_idx;
1461+
/* in back branches, put this at the end to avoid ABI break: */
1462+
List**exprstatelists;
14541463
}ValuesScanState;
14551464

14561465
/* ----------------

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,33 @@ where s.i < 10 and (select val.x) < 110;
740740
10
741741
(17 rows)
742742

743+
-- another variant of that (bug #16213)
744+
explain (verbose, costs off)
745+
select * from
746+
(values
747+
(3 not in (select * from (values (1), (2)) ss1)),
748+
(false)
749+
) ss;
750+
QUERY PLAN
751+
----------------------------------------
752+
Values Scan on "*VALUES*"
753+
Output: "*VALUES*".column1
754+
SubPlan 1
755+
-> Values Scan on "*VALUES*_1"
756+
Output: "*VALUES*_1".column1
757+
(5 rows)
758+
759+
select * from
760+
(values
761+
(3 not in (select * from (values (1), (2)) ss1)),
762+
(false)
763+
) ss;
764+
column1
765+
---------
766+
t
767+
f
768+
(2 rows)
769+
743770
--
744771
-- Check sane behavior with nested IN SubLinks
745772
--

‎src/test/regress/sql/subselect.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,20 @@ select val.x
422422
)as val(x)
423423
wheres.i<10and (selectval.x)<110;
424424

425+
-- another variant of that (bug #16213)
426+
explain (verbose, costs off)
427+
select*from
428+
(values
429+
(3 notin (select*from (values (1), (2)) ss1)),
430+
(false)
431+
) ss;
432+
433+
select*from
434+
(values
435+
(3 notin (select*from (values (1), (2)) ss1)),
436+
(false)
437+
) ss;
438+
425439
--
426440
-- Check sane behavior with nested IN SubLinks
427441
--

0 commit comments

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