From: Tom Lane Date: Sat, 26 Jul 2008 19:15:35 +0000 (+0000) Subject: As noted by Andrew Gierth, there's really no need any more to force a junk X-Git-Tag: REL8_4_BETA1~1130 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a77eaa6a95009a3441e0d475d1980259d45da072;p=postgresql As noted by Andrew Gierth, there's really no need any more to force a junk filter to be used when INSERT or SELECT INTO has a plan that returns raw disk tuples. The virtual-tuple-slot optimizations that were put in place awhile ago mean that ExecInsert has to do ExecMaterializeSlot, and that already copies the tuple if it's raw (and does so more efficiently than a junk filter, too). So get rid of that logic. This in turn means that we can throw away ExecMayReturnRawTuples, which wasn't used for any other purpose, and was always a kluge anyway. In passing, move a couple of SELECT-INTO-specific fields out of EState and into the private state of the SELECT INTO DestReceiver, as was foreseen in an old comment there. Also make intorel_receive use ExecMaterializeSlot not ExecCopySlotTuple, for consistency with ExecInsert and to possibly save a tuple copy step in some cases. --- diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c index b637b5a08f..154301a67f 100644 --- a/src/backend/executor/execAmi.c +++ b/src/backend/executor/execAmi.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/executor/execAmi.c,v 1.95 2008/07/10 01:17:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execAmi.c,v 1.96 2008/07/26 19:15:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -424,72 +424,3 @@ ExecSupportsBackwardScan(Plan *node) return false; } } - -/* - * ExecMayReturnRawTuples - * Check whether a plan tree may return "raw" disk tuples (that is, - * pointers to original data in disk buffers, as opposed to temporary - * tuples constructed by projection steps). In the case of Append, - * some subplans may return raw tuples and others projected tuples; - * we return "true" if any of the returned tuples could be raw. - * - * This must be passed an already-initialized planstate tree, because we - * need to look at the results of ExecAssignScanProjectionInfo(). - */ -bool -ExecMayReturnRawTuples(PlanState *node) -{ - /* - * At a table scan node, we check whether ExecAssignScanProjectionInfo - * decided to do projection or not. Most non-scan nodes always project - * and so we can return "false" immediately. For nodes that don't project - * but just pass up input tuples, we have to recursively examine the input - * plan node. - * - * Note: Hash and Material are listed here because they sometimes return - * an original input tuple, not a copy. But Sort and SetOp never return - * an original tuple, so they can be treated like projecting nodes. - */ - switch (nodeTag(node)) - { - /* Table scan nodes */ - case T_SeqScanState: - case T_IndexScanState: - case T_BitmapHeapScanState: - case T_TidScanState: - if (node->ps_ProjInfo == NULL) - return true; - break; - - case T_SubqueryScanState: - /* If not projecting, look at input plan */ - if (node->ps_ProjInfo == NULL) - return ExecMayReturnRawTuples(((SubqueryScanState *) node)->subplan); - break; - - /* Non-projecting nodes */ - case T_HashState: - case T_MaterialState: - case T_UniqueState: - case T_LimitState: - return ExecMayReturnRawTuples(node->lefttree); - - case T_AppendState: - { - AppendState *appendstate = (AppendState *) node; - int j; - - for (j = 0; j < appendstate->as_nplans; j++) - { - if (ExecMayReturnRawTuples(appendstate->appendplans[j])) - return true; - } - break; - } - - /* All projecting node types come here */ - default: - break; - } - return false; -} diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 93f7804670..4a4c918857 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.310 2008/07/18 18:23:46 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.311 2008/07/26 19:15:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -694,9 +694,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* * Initialize the junk filter if needed. SELECT and INSERT queries need a - * filter if there are any junk attrs in the tlist. INSERT and SELECT - * INTO also need a filter if the plan may return raw disk tuples (else - * heap_insert will be scribbling on the source relation!). UPDATE and + * filter if there are any junk attrs in the tlist. UPDATE and * DELETE always need a filter, since there's always a junk 'ctid' * attribute present --- no need to look first. */ @@ -718,10 +716,6 @@ InitPlan(QueryDesc *queryDesc, int eflags) break; } } - if (!junk_filter_needed && - (operation == CMD_INSERT || estate->es_select_into) && - ExecMayReturnRawTuples(planstate)) - junk_filter_needed = true; break; case CMD_UPDATE: case CMD_DELETE: @@ -2424,8 +2418,6 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq) epqstate->es_result_relation_info = estate->es_result_relation_info; epqstate->es_junkFilter = estate->es_junkFilter; /* es_trig_target_relations must NOT be copied */ - epqstate->es_into_relation_descriptor = estate->es_into_relation_descriptor; - epqstate->es_into_relation_use_wal = estate->es_into_relation_use_wal; epqstate->es_param_list_info = estate->es_param_list_info; if (estate->es_plannedstmt->nParamExec > 0) epqstate->es_param_exec_vals = (ParamExecData *) @@ -2559,15 +2551,14 @@ ExecGetActivePlanTree(QueryDesc *queryDesc) * * We implement SELECT INTO by diverting SELECT's normal output with * a specialized DestReceiver type. - * - * TODO: remove some of the INTO-specific cruft from EState, and keep - * it in the DestReceiver instead. */ typedef struct { DestReceiver pub; /* publicly-known function pointers */ EState *estate; /* EState we are working with */ + Relation rel; /* Relation to write to */ + bool use_wal; /* do we need to WAL-log our writes? */ } DR_intorel; /* @@ -2692,15 +2683,6 @@ OpenIntoRel(QueryDesc *queryDesc) */ intoRelationDesc = heap_open(intoRelationId, AccessExclusiveLock); - /* use_wal off requires rd_targblock be initially invalid */ - Assert(intoRelationDesc->rd_targblock == InvalidBlockNumber); - - /* - * We can skip WAL-logging the insertions, unless PITR is in use. - */ - estate->es_into_relation_use_wal = XLogArchivingActive(); - estate->es_into_relation_descriptor = intoRelationDesc; - /* * Now replace the query's DestReceiver with one for SELECT INTO */ @@ -2708,6 +2690,15 @@ OpenIntoRel(QueryDesc *queryDesc) myState = (DR_intorel *) queryDesc->dest; Assert(myState->pub.mydest == DestIntoRel); myState->estate = estate; + + /* + * We can skip WAL-logging the insertions, unless PITR is in use. + */ + myState->use_wal = XLogArchivingActive(); + myState->rel = intoRelationDesc; + + /* use_wal off requires rd_targblock be initially invalid */ + Assert(intoRelationDesc->rd_targblock == InvalidBlockNumber); } /* @@ -2716,19 +2707,19 @@ OpenIntoRel(QueryDesc *queryDesc) static void CloseIntoRel(QueryDesc *queryDesc) { - EState *estate = queryDesc->estate; + DR_intorel *myState = (DR_intorel *) queryDesc->dest; /* OpenIntoRel might never have gotten called */ - if (estate->es_into_relation_descriptor) + if (myState && myState->pub.mydest == DestIntoRel && myState->rel) { /* If we skipped using WAL, must heap_sync before commit */ - if (!estate->es_into_relation_use_wal) - heap_sync(estate->es_into_relation_descriptor); + if (!myState->use_wal) + heap_sync(myState->rel); /* close rel, but keep lock until commit */ - heap_close(estate->es_into_relation_descriptor, NoLock); + heap_close(myState->rel, NoLock); - estate->es_into_relation_descriptor = NULL; + myState->rel = NULL; } } @@ -2736,13 +2727,13 @@ CloseIntoRel(QueryDesc *queryDesc) * CreateIntoRelDestReceiver -- create a suitable DestReceiver object * * Since CreateDestReceiver doesn't accept the parameters we'd need, - * we just leave the private fields empty here. OpenIntoRel will + * we just leave the private fields zeroed here. OpenIntoRel will * fill them in. */ DestReceiver * CreateIntoRelDestReceiver(void) { - DR_intorel *self = (DR_intorel *) palloc(sizeof(DR_intorel)); + DR_intorel *self = (DR_intorel *) palloc0(sizeof(DR_intorel)); self->pub.receiveSlot = intorel_receive; self->pub.rStartup = intorel_startup; @@ -2750,8 +2741,6 @@ CreateIntoRelDestReceiver(void) self->pub.rDestroy = intorel_destroy; self->pub.mydest = DestIntoRel; - self->estate = NULL; - return (DestReceiver *) self; } @@ -2771,21 +2760,22 @@ static void intorel_receive(TupleTableSlot *slot, DestReceiver *self) { DR_intorel *myState = (DR_intorel *) self; - EState *estate = myState->estate; HeapTuple tuple; - tuple = ExecCopySlotTuple(slot); + /* + * get the heap tuple out of the tuple table slot, making sure we have a + * writable copy + */ + tuple = ExecMaterializeSlot(slot); - heap_insert(estate->es_into_relation_descriptor, + heap_insert(myState->rel, tuple, - estate->es_output_cid, - estate->es_into_relation_use_wal, + myState->estate->es_output_cid, + myState->use_wal, false); /* never any point in using FSM */ /* We know this is a newly created relation, so there are no indexes */ - heap_freetuple(tuple); - IncrAppended(); } diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 6dad21e306..f53f9d12e9 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.155 2008/03/26 21:10:38 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.156 2008/07/26 19:15:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -190,9 +190,6 @@ CreateExecutorState(void) estate->es_trig_target_relations = NIL; estate->es_trig_tuple_slot = NULL; - estate->es_into_relation_descriptor = NULL; - estate->es_into_relation_use_wal = false; - estate->es_param_list_info = NULL; estate->es_param_exec_vals = NULL; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 71cb94d4c5..7788a22b76 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.148 2008/07/18 18:23:47 tgl Exp $ + * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.149 2008/07/26 19:15:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -75,7 +75,6 @@ extern void ExecMarkPos(PlanState *node); extern void ExecRestrPos(PlanState *node); extern bool ExecSupportsMarkRestore(NodeTag plantype); extern bool ExecSupportsBackwardScan(Plan *node); -extern bool ExecMayReturnRawTuples(PlanState *node); /* * prototypes from functions in execCurrent.c diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index f21a0528e7..96b7a16084 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.184 2008/06/19 00:46:06 alvherre Exp $ + * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.185 2008/07/26 19:15:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -328,10 +328,6 @@ typedef struct EState List *es_trig_target_relations; /* trigger-only ResultRelInfos */ TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */ - /* Stuff used for SELECT INTO: */ - Relation es_into_relation_descriptor; - bool es_into_relation_use_wal; - /* Parameter info: */ ParamListInfo es_param_list_info; /* values of external params */ ParamExecData *es_param_exec_vals; /* values of internal params */