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

Commit167fd02

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 parente3154aa commit167fd02

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
#include"utils/expandeddatum.h"
2930

3031

@@ -49,7 +50,7 @@ ValuesNext(ValuesScanState *node)
4950
EState*estate;
5051
ExprContext*econtext;
5152
ScanDirectiondirection;
52-
List*exprlist;
53+
intcurr_idx;
5354

5455
/*
5556
* get information from the estate and scan state
@@ -66,19 +67,11 @@ ValuesNext(ValuesScanState *node)
6667
{
6768
if (node->curr_idx<node->array_len)
6869
node->curr_idx++;
69-
if (node->curr_idx<node->array_len)
70-
exprlist=node->exprlists[node->curr_idx];
71-
else
72-
exprlist=NIL;
7370
}
7471
else
7572
{
7673
if (node->curr_idx >=0)
7774
node->curr_idx--;
78-
if (node->curr_idx >=0)
79-
exprlist=node->exprlists[node->curr_idx];
80-
else
81-
exprlist=NIL;
8275
}
8376

8477
/*
@@ -89,11 +82,12 @@ ValuesNext(ValuesScanState *node)
8982
*/
9083
ExecClearTuple(slot);
9184

92-
if (exprlist)
85+
curr_idx=node->curr_idx;
86+
if (curr_idx >=0&&curr_idx<node->array_len)
9387
{
88+
List*exprlist=node->exprlists[curr_idx];
89+
List*exprstatelist=node->exprstatelists[curr_idx];
9490
MemoryContextoldContext;
95-
List*oldsubplans;
96-
List*exprstatelist;
9791
Datum*values;
9892
bool*isnull;
9993
Form_pg_attribute*att;
@@ -108,30 +102,28 @@ ValuesNext(ValuesScanState *node)
108102
ReScanExprContext(econtext);
109103

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

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

136128
/* parser should have checked all sublists are the same length */
137129
Assert(list_length(exprstatelist)==slot->tts_tupleDescriptor->natts);
@@ -272,13 +264,38 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags)
272264
scanstate->curr_idx=-1;
273265
scanstate->array_len=list_length(node->values_lists);
274266

275-
/* convert list of sublists into array of sublists for easy addressing */
267+
/*
268+
* Convert the list of expression sublists into an array for easier
269+
* addressing at runtime. Also, detect whether any sublists contain
270+
* SubPlans; for just those sublists, go ahead and do expression
271+
* initialization. (This avoids problems with SubPlans wanting to connect
272+
* themselves up to the outer plan tree. Notably, EXPLAIN won't see the
273+
* subplans otherwise; also we will have troubles with dangling pointers
274+
* and/or leaked resources if we try to handle SubPlans the same as
275+
* simpler expressions.)
276+
*/
276277
scanstate->exprlists= (List**)
277278
palloc(scanstate->array_len*sizeof(List*));
279+
scanstate->exprstatelists= (List**)
280+
palloc0(scanstate->array_len*sizeof(List*));
278281
i=0;
279282
foreach(vtl,node->values_lists)
280283
{
281-
scanstate->exprlists[i++]= (List*)lfirst(vtl);
284+
List*exprs=castNode(List,lfirst(vtl));
285+
286+
scanstate->exprlists[i]=exprs;
287+
288+
/*
289+
* We can avoid the cost of a contain_subplans() scan in the simple
290+
* case where there are no SubPlans anywhere.
291+
*/
292+
if (estate->es_subplanstates&&
293+
contain_subplans((Node*)exprs))
294+
{
295+
scanstate->exprstatelists[i]=ExecInitExprList(exprs,
296+
&scanstate->ss.ps);
297+
}
298+
i++;
282299
}
283300

284301
/*

‎src/include/nodes/execnodes.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,14 +1444,21 @@ typedef struct FunctionScanState
14441444
*
14451445
*rowcontextper-expression-list context
14461446
*exprlistsarray of expression lists being evaluated
1447-
*array_lensize of array
1447+
*exprstatelistsarray of expression state lists, for SubPlans only
1448+
*array_lensize of above arrays
14481449
*curr_idxcurrent array index (0-based)
14491450
*
14501451
*Note: ss.ps.ps_ExprContext is used to evaluate any qual or projection
14511452
*expressions attached to the node. We create a second ExprContext,
14521453
*rowcontext, in which to build the executor expression state for each
14531454
*Values sublist. Resetting this context lets us get rid of expression
14541455
*state for each row, avoiding major memory leakage over a long values list.
1456+
*However, that doesn't work for sublists containing SubPlans, because a
1457+
*SubPlan has to be connected up to the outer plan tree to work properly.
1458+
*Therefore, for only those sublists containing SubPlans, we do expression
1459+
*state construction at executor start, and store those pointers in
1460+
*exprstatelists[]. NULL entries in that array correspond to simple
1461+
*subexpressions that are handled as described above.
14551462
* ----------------
14561463
*/
14571464
typedefstructValuesScanState
@@ -1461,6 +1468,8 @@ typedef struct ValuesScanState
14611468
List**exprlists;
14621469
intarray_len;
14631470
intcurr_idx;
1471+
/* in back branches, put this at the end to avoid ABI break: */
1472+
List**exprstatelists;
14641473
}ValuesScanState;
14651474

14661475
/* ----------------

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

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

873+
-- another variant of that (bug #16213)
874+
explain (verbose, costs off)
875+
select * from
876+
(values
877+
(3 not in (select * from (values (1), (2)) ss1)),
878+
(false)
879+
) ss;
880+
QUERY PLAN
881+
----------------------------------------
882+
Values Scan on "*VALUES*"
883+
Output: "*VALUES*".column1
884+
SubPlan 1
885+
-> Values Scan on "*VALUES*_1"
886+
Output: "*VALUES*_1".column1
887+
(5 rows)
888+
889+
select * from
890+
(values
891+
(3 not in (select * from (values (1), (2)) ss1)),
892+
(false)
893+
) ss;
894+
column1
895+
---------
896+
t
897+
f
898+
(2 rows)
899+
873900
--
874901
-- Check sane behavior with nested IN SubLinks
875902
--

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

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

485+
-- another variant of that (bug #16213)
486+
explain (verbose, costs off)
487+
select*from
488+
(values
489+
(3 notin (select*from (values (1), (2)) ss1)),
490+
(false)
491+
) ss;
492+
493+
select*from
494+
(values
495+
(3 notin (select*from (values (1), (2)) ss1)),
496+
(false)
497+
) ss;
498+
485499
--
486500
-- Check sane behavior with nested IN SubLinks
487501
--

0 commit comments

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