Avoid extra tuple table slots in CustomScan nodes

The `CustomScan` node is oddly hard-coded to use fixed tuple table
slots in its initialization, thus expecting both the in and out slots
(scan and result slots) to always have the same type (underlying ops).

However, when implementing an append-type node on `CustomScan`, the
children that supply tuples can have slots of any type. In PG12, the
regular `Append` node just passes on these slots, signaling this by
setting `resultopsfixed = false`.

This change adopts the behavior of `Append` for out custom
`ChunkAppend` and `ConstraintAwareAppend` nodes.

An alternative would be to copy the contents of the child's slot into
the `CustomScan`'s own scan slot to get it to the "expected" type. But
this would require materializing the tuple, which seems unnecessary.
This commit is contained in:
Erik Nordström 2020-02-24 21:58:39 +01:00 committed by Erik Nordström
parent 7a0d6e7c23
commit f284a691b2
4 changed files with 53 additions and 59 deletions

View File

@ -3,7 +3,6 @@
* Please see the included NOTICE for copyright information and
* LICENSE-APACHE for a copy of the license.
*/
#include <postgres.h>
#include <fmgr.h>
#include <miscadmin.h>
@ -217,6 +216,29 @@ chunk_append_begin(CustomScanState *node, EState *estate, int eflags)
ListCell *lc;
int i;
#if PG12_GE
/* CustomScan hard-codes the scan and result tuple slot to a fixed
* TTSOpsVirtual ops (meaning it expects the slot ops of the child tuple to
* also have this type). Oddly, when reading slots from subscan nodes
* (children), there is no knowing what tuple slot ops the child slot will
* have (e.g., for ChunkAppend it is common that the child is a
* seqscan/indexscan that produces a TTSOpsBufferHeapTuple
* slot). Unfortunately, any mismatch between slot types when projecting is
* asserted by PostgreSQL. To avoid this issue, we mark the scanops as
* non-fixed and reinitialize the projection state with this new setting.
*
* Alternatively, we could copy the child tuple into the scan slot to get
* the expected ops before projection, but this would require materializing
* and copying the tuple unnecessarily.
*/
node->ss.ps.scanopsfixed = false;
/* Since we sometimes return the scan slot directly from the subnode, the
* result slot is not fixed either. */
node->ss.ps.resultopsfixed = false;
ExecAssignScanProjectionInfoWithVarno(&node->ss, INDEX_VAR);
#endif
initialize_constraints(state, lthird(cscan->custom_private));
if (state->startup_exclusion)
@ -224,16 +246,6 @@ chunk_append_begin(CustomScanState *node, EState *estate, int eflags)
state->num_subplans = list_length(state->filtered_subplans);
#if PG12_GE
ExecInitResultTupleSlotTL(&node->ss.ps, TTSOpsVirtualP);
/* node returns slots from each of its subnodes, therefore not fixed */
node->ss.ps.resultopsset = true;
node->ss.ps.resultopsfixed = false;
state->slot = MakeSingleTupleTableSlot(NULL, TTSOpsVirtualP);
#endif
if (state->num_subplans == 0)
{
state->current = NO_MATCHING_SUBPLANS;
@ -355,11 +367,12 @@ static TupleTableSlot *
chunk_append_exec(CustomScanState *node)
{
ChunkAppendState *state = (ChunkAppendState *) node;
TupleTableSlot *subslot;
ExprContext *econtext = node->ss.ps.ps_ExprContext;
ProjectionInfo *projinfo = node->ss.ps.ps_ProjInfo;
TupleTableSlot *subslot;
#if PG96
TupleTableSlot *resultslot;
ExprDoneCond isDone;
ExprDoneCond isdone;
#endif
if (state->current == INVALID_SUBPLAN_INDEX)
@ -368,9 +381,9 @@ chunk_append_exec(CustomScanState *node)
#if PG96
if (node->ss.ps.ps_TupFromTlist)
{
resultslot = ExecProject(node->ss.ps.ps_ProjInfo, &isDone);
resultslot = ExecProject(projinfo, &isdone);
if (isDone == ExprMultipleResult)
if (isdone == ExprMultipleResult)
return resultslot;
node->ss.ps.ps_TupFromTlist = false;
@ -387,7 +400,6 @@ chunk_append_exec(CustomScanState *node)
return ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
Assert(state->current >= 0 && state->current < state->num_subplans);
subnode = state->subplanstates[state->current];
/*
@ -397,43 +409,29 @@ chunk_append_exec(CustomScanState *node)
if (!TupIsNull(subslot))
{
#if PG12_GE
/* convert to Virtual tuple, so the tts_ops don't conflict */
if (state->slot->tts_tupleDescriptor != subslot->tts_tupleDescriptor)
ExecSetSlotDescriptor(state->slot, subslot->tts_tupleDescriptor);
ExecCopySlot(state->slot, subslot);
subslot = state->slot;
#endif
/*
* If the subplan gave us something check if we need
* to do projection otherwise return as is.
*/
if (node->ss.ps.ps_ProjInfo == NULL)
if (projinfo == NULL)
return subslot;
ResetExprContext(econtext);
econtext->ecxt_scantuple = subslot;
#if PG96
resultslot = ExecProject(node->ss.ps.ps_ProjInfo, &isDone);
resultslot = ExecProject(projinfo, &isdone);
if (isDone != ExprEndResult)
if (isdone != ExprEndResult)
{
node->ss.ps.ps_TupFromTlist = (isDone == ExprMultipleResult);
node->ss.ps.ps_TupFromTlist = (isdone == ExprMultipleResult);
return resultslot;
}
#else
return ExecProject(node->ss.ps.ps_ProjInfo);
return ExecProject(projinfo);
#endif
}
#if PG12_GE
ExecClearTuple(state->slot);
#endif
state->choose_next_subplan(state);
/* loop back and try to get a tuple from the new subplan */
@ -584,10 +582,6 @@ chunk_append_end(CustomScanState *node)
{
ExecEndNode(state->subplanstates[i]);
}
#if PG12_GE
ExecDropSingleTupleTableSlot(state->slot);
#endif
}
/*
@ -813,7 +807,6 @@ ca_get_relation_constraints(Oid relationObjectId, Index varno, bool include_notn
/*
* We assume the relation has already been safely locked.
*/
// FIXME this lock should not be needed...
relation = table_open(relationObjectId, AccessShareLock);
constr = relation->rd_att->constr;

View File

@ -63,10 +63,6 @@ typedef struct ChunkAppendState
ParallelContext *pcxt;
ParallelChunkAppendState *pstate;
void (*choose_next_subplan)(struct ChunkAppendState *);
#if PG12_GE
TupleTableSlot *slot;
#endif
} ChunkAppendState;
extern Node *ts_chunk_append_state_create(CustomScan *cscan);

View File

@ -143,7 +143,26 @@ ca_append_begin(CustomScanState *node, EState *estate, int eflags)
};
#if PG12_GE
state->slot = MakeSingleTupleTableSlot(NULL, TTSOpsVirtualP);
/* CustomScan hard-codes the scan and result tuple slot to a fixed
* TTSOpsVirtual ops (meaning it expects the slot ops of the child tuple to
* also have this type). Oddly, when reading slots from subscan nodes
* (children), there is no knowing what tuple slot ops the child slot will
* have (e.g., for ChunkAppend it is common that the child is a
* seqscan/indexscan that produces a TTSOpsBufferHeapTuple
* slot). Unfortunately, any mismatch between slot types when projecting is
* asserted by PostgreSQL. To avoid this issue, we mark the scanops as
* non-fixed and reinitialize the projection state with this new setting.
*
* Alternatively, we could copy the child tuple into the scan slot to get
* the expected ops before projection, but this would require materializing
* and copying the tuple unnecessarily.
*/
node->ss.ps.scanopsfixed = false;
/* Since we sometimes return the scan slot directly from the subnode, the
* result slot is not fixed either. */
node->ss.ps.resultopsfixed = false;
ExecAssignScanProjectionInfoWithVarno(&node->ss, INDEX_VAR);
#endif
switch (nodeTag(subplan))
@ -292,14 +311,6 @@ ca_append_exec(CustomScanState *node)
if (TupIsNull(subslot))
return NULL;
#if PG12_GE
if (state->slot->tts_tupleDescriptor != subslot->tts_tupleDescriptor)
ExecSetSlotDescriptor(state->slot, subslot->tts_tupleDescriptor);
ExecCopySlot(state->slot, subslot);
subslot = state->slot;
#endif
if (!node->ss.ps.ps_ProjInfo)
return subslot;
@ -326,9 +337,6 @@ ca_append_end(CustomScanState *node)
{
ExecEndNode(linitial(node->custom_ps));
}
#if PG12_GE
ExecDropSingleTupleTableSlot(((ConstraintAwareAppendState *) node)->slot);
#endif
}
static void

View File

@ -19,9 +19,6 @@ typedef struct ConstraintAwareAppendState
CustomScanState csstate;
Plan *subplan;
Size num_append_subplans;
#if PG12_GE
TupleTableSlot *slot;
#endif
} ConstraintAwareAppendState;
typedef struct Hypertable Hypertable;