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

Commit41c6f9d

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 parent15cac3a commit41c6f9d

File tree

4 files changed

+118
-43
lines changed

4 files changed

+118
-43
lines changed

‎src/backend/executor/nodeValuesscan.c

Lines changed: 68 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include"executor/executor.h"
2727
#include"executor/nodeValuesscan.h"
2828
#include"jit/jit.h"
29+
#include"optimizer/clauses.h"
2930
#include"utils/expandeddatum.h"
3031

3132

@@ -50,7 +51,7 @@ ValuesNext(ValuesScanState *node)
5051
EState*estate;
5152
ExprContext*econtext;
5253
ScanDirectiondirection;
53-
List*exprlist;
54+
intcurr_idx;
5455

5556
/*
5657
* get information from the estate and scan state
@@ -67,19 +68,11 @@ ValuesNext(ValuesScanState *node)
6768
{
6869
if (node->curr_idx<node->array_len)
6970
node->curr_idx++;
70-
if (node->curr_idx<node->array_len)
71-
exprlist=node->exprlists[node->curr_idx];
72-
else
73-
exprlist=NIL;
7471
}
7572
else
7673
{
7774
if (node->curr_idx >=0)
7875
node->curr_idx--;
79-
if (node->curr_idx >=0)
80-
exprlist=node->exprlists[node->curr_idx];
81-
else
82-
exprlist=NIL;
8376
}
8477

8578
/*
@@ -90,16 +83,16 @@ ValuesNext(ValuesScanState *node)
9083
*/
9184
ExecClearTuple(slot);
9285

93-
if (exprlist)
86+
curr_idx=node->curr_idx;
87+
if (curr_idx >=0&&curr_idx<node->array_len)
9488
{
89+
List*exprlist=node->exprlists[curr_idx];
90+
List*exprstatelist=node->exprstatelists[curr_idx];
9591
MemoryContextoldContext;
96-
List*oldsubplans;
97-
List*exprstatelist;
9892
Datum*values;
9993
bool*isnull;
10094
ListCell*lc;
10195
intresind;
102-
intsaved_jit_flags;
10396

10497
/*
10598
* Get rid of any prior cycle's leftovers. We use ReScanExprContext
@@ -109,38 +102,32 @@ ValuesNext(ValuesScanState *node)
109102
ReScanExprContext(econtext);
110103

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

119109
/*
120-
* The expressions might contain SubPlans (this is currently only
121-
* possible if there's a sub-select containing a LATERAL reference,
122-
* otherwise sub-selects in a VALUES list should be InitPlans). Those
123-
* subplans will want to hook themselves into our subPlan list, which
124-
* would result in a corrupted list after we delete the eval state. We
125-
* can work around this by saving and restoring the subPlan list.
126-
* (There's no need for the functionality that would be enabled by
127-
* having the list entries, since the SubPlans aren't going to be
128-
* re-executed anyway.)
129-
*/
130-
oldsubplans=node->ss.ps.subPlan;
131-
node->ss.ps.subPlan=NIL;
132-
133-
/*
134-
* As the expressions are only ever used once, disable JIT for them.
135-
* This is worthwhile because it's common to insert significant
136-
* amounts of data via VALUES().
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.
137116
*/
138-
saved_jit_flags=econtext->ecxt_estate->es_jit_flags;
139-
econtext->ecxt_estate->es_jit_flags=PGJIT_NONE;
140-
exprstatelist=ExecInitExprList(exprlist,&node->ss.ps);
141-
econtext->ecxt_estate->es_jit_flags=saved_jit_flags;
142-
143-
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+
* Note that passing parent = NULL also disables JIT compilation
126+
* of the expressions, which is a win, because they're only going
127+
* to be used once under normal circumstances.
128+
*/
129+
exprstatelist=ExecInitExprList(exprlist,NULL);
130+
}
144131

145132
/* parser should have checked all sublists are the same length */
146133
Assert(list_length(exprstatelist)==slot->tts_tupleDescriptor->natts);
@@ -281,13 +268,52 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags)
281268
scanstate->curr_idx=-1;
282269
scanstate->array_len=list_length(node->values_lists);
283270

284-
/* convert list of sublists into array of sublists for easy addressing */
271+
/*
272+
* Convert the list of expression sublists into an array for easier
273+
* addressing at runtime. Also, detect whether any sublists contain
274+
* SubPlans; for just those sublists, go ahead and do expression
275+
* initialization. (This avoids problems with SubPlans wanting to connect
276+
* themselves up to the outer plan tree. Notably, EXPLAIN won't see the
277+
* subplans otherwise; also we will have troubles with dangling pointers
278+
* and/or leaked resources if we try to handle SubPlans the same as
279+
* simpler expressions.)
280+
*/
285281
scanstate->exprlists= (List**)
286282
palloc(scanstate->array_len*sizeof(List*));
283+
scanstate->exprstatelists= (List**)
284+
palloc0(scanstate->array_len*sizeof(List*));
287285
i=0;
288286
foreach(vtl,node->values_lists)
289287
{
290-
scanstate->exprlists[i++]= (List*)lfirst(vtl);
288+
List*exprs=castNode(List,lfirst(vtl));
289+
290+
scanstate->exprlists[i]=exprs;
291+
292+
/*
293+
* We can avoid the cost of a contain_subplans() scan in the simple
294+
* case where there are no SubPlans anywhere.
295+
*/
296+
if (estate->es_subplanstates&&
297+
contain_subplans((Node*)exprs))
298+
{
299+
intsaved_jit_flags;
300+
301+
/*
302+
* As these expressions are only used once, disable JIT for them.
303+
* This is worthwhile because it's common to insert significant
304+
* amounts of data via VALUES(). Note that this doesn't prevent
305+
* use of JIT *within* a subplan, since that's initialized
306+
* separately; this just affects the upper-level subexpressions.
307+
*/
308+
saved_jit_flags=estate->es_jit_flags;
309+
estate->es_jit_flags=PGJIT_NONE;
310+
311+
scanstate->exprstatelists[i]=ExecInitExprList(exprs,
312+
&scanstate->ss.ps);
313+
314+
estate->es_jit_flags=saved_jit_flags;
315+
}
316+
i++;
291317
}
292318

293319
returnscanstate;

‎src/include/nodes/execnodes.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1670,21 +1670,29 @@ typedef struct FunctionScanState
16701670
*
16711671
*rowcontextper-expression-list context
16721672
*exprlistsarray of expression lists being evaluated
1673-
*array_lensize of array
1673+
*exprstatelistsarray of expression state lists, for SubPlans only
1674+
*array_lensize of above arrays
16741675
*curr_idxcurrent array index (0-based)
16751676
*
16761677
*Note: ss.ps.ps_ExprContext is used to evaluate any qual or projection
16771678
*expressions attached to the node. We create a second ExprContext,
16781679
*rowcontext, in which to build the executor expression state for each
16791680
*Values sublist. Resetting this context lets us get rid of expression
16801681
*state for each row, avoiding major memory leakage over a long values list.
1682+
*However, that doesn't work for sublists containing SubPlans, because a
1683+
*SubPlan has to be connected up to the outer plan tree to work properly.
1684+
*Therefore, for only those sublists containing SubPlans, we do expression
1685+
*state construction at executor start, and store those pointers in
1686+
*exprstatelists[]. NULL entries in that array correspond to simple
1687+
*subexpressions that are handled as described above.
16811688
* ----------------
16821689
*/
16831690
typedefstructValuesScanState
16841691
{
16851692
ScanStatess;/* its first field is NodeTag */
16861693
ExprContext*rowcontext;
16871694
List**exprlists;
1695+
List**exprstatelists;
16881696
intarray_len;
16891697
intcurr_idx;
16901698
}ValuesScanState;

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

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

929+
-- another variant of that (bug #16213)
930+
explain (verbose, costs off)
931+
select * from
932+
(values
933+
(3 not in (select * from (values (1), (2)) ss1)),
934+
(false)
935+
) ss;
936+
QUERY PLAN
937+
----------------------------------------
938+
Values Scan on "*VALUES*"
939+
Output: "*VALUES*".column1
940+
SubPlan 1
941+
-> Values Scan on "*VALUES*_1"
942+
Output: "*VALUES*_1".column1
943+
(5 rows)
944+
945+
select * from
946+
(values
947+
(3 not in (select * from (values (1), (2)) ss1)),
948+
(false)
949+
) ss;
950+
column1
951+
---------
952+
t
953+
f
954+
(2 rows)
955+
929956
--
930957
-- Check sane behavior with nested IN SubLinks
931958
--

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

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

521+
-- another variant of that (bug #16213)
522+
explain (verbose, costs off)
523+
select*from
524+
(values
525+
(3 notin (select*from (values (1), (2)) ss1)),
526+
(false)
527+
) ss;
528+
529+
select*from
530+
(values
531+
(3 notin (select*from (values (1), (2)) ss1)),
532+
(false)
533+
) ss;
534+
521535
--
522536
-- Check sane behavior with nested IN SubLinks
523537
--

0 commit comments

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