diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-01-17 16:17:17 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-01-17 16:17:36 -0500 |
commit | 45f03cfa56c88a3c662436027f076ad2b667287e (patch) | |
tree | dce3b3b87fa63e674943214f3687625459d4e5df /src/include/nodes/execnodes.h | |
parent | cdb14154bb00e711152f4011417b3e44ea85adea (diff) |
Repair more failures with SubPlans in multi-row VALUES lists.
Commit 9b63c13f0 turns out to have been fundamentally misguided:
the parent node's subPlan list is by no means the only way in which
a child SubPlan node can be hooked into the outer execution state.
As shown in bug #16213 from Matt Jibson, we can also get short-lived
tuple table slots added to the outer es_tupleTable list. At this point
I have little faith that there aren't other possible connections as
well; the long time it took to notice this problem shows that this
isn't a heavily-exercised situation.
Therefore, revert that fix, returning to the coding that passed a
NULL parent plan pointer down to the transiently-built subexpressions.
That gives us a pretty good guarantee that they won't hook into the
outer executor state in any way. But then we need some other solution
to make SubPlans work. Adopt the solution speculated about in the
previous commit's log message: do expression initialization at plan
startup for just those VALUES rows containing SubPlans, abandoning the
goal of reclaiming memory intra-query for those rows. In practice it
seems unlikely that queries containing a vast number of VALUES rows
would be using SubPlans in them, so this should not give up much.
(BTW, this test case also refutes my claim in connection with the prior
commit that the issue only arises with use of LATERAL. That was just
wrong: 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
Diffstat (limited to 'src/include/nodes/execnodes.h')
-rw-r--r-- | src/include/nodes/execnodes.h | 11 |
1 files changed, 10 insertions, 1 deletions
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7621a171671..f303c951530 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1524,7 +1524,8 @@ typedef struct FunctionScanState * * rowcontext per-expression-list context * exprlists array of expression lists being evaluated - * array_len size of array + * exprstatelists array of expression state lists, for SubPlans only + * array_len size of above arrays * curr_idx current array index (0-based) * * Note: ss.ps.ps_ExprContext is used to evaluate any qual or projection @@ -1532,6 +1533,12 @@ typedef struct FunctionScanState * rowcontext, in which to build the executor expression state for each * Values sublist. Resetting this context lets us get rid of expression * state for each row, avoiding major memory leakage over a long values list. + * However, that doesn't work for sublists containing SubPlans, because a + * SubPlan has to be connected up to the outer plan tree to work properly. + * Therefore, for only those sublists containing SubPlans, we do expression + * state construction at executor start, and store those pointers in + * exprstatelists[]. NULL entries in that array correspond to simple + * subexpressions that are handled as described above. * ---------------- */ typedef struct ValuesScanState @@ -1541,6 +1548,8 @@ typedef struct ValuesScanState List **exprlists; int array_len; int curr_idx; + /* in back branches, put this at the end to avoid ABI break: */ + List **exprstatelists; } ValuesScanState; /* ---------------- |