mirror of
https://git.postgresql.org/git/postgresql.git
synced 2025-02-23 19:39:53 +08:00
Fix oversight in optimization that avoids an unnecessary projection step
when scanning a table that we need all the columns from. In case of SELECT INTO, we have to check that the hasoids flag matches the desired output type, too. Per report from Mike Mascari.
This commit is contained in:
Tom Lane
parent
5a2a8b9e01
commit
94db74f370
@ -26,7 +26,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.220 2003/10/01 21:30:52 tgl Exp $
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.220.2.1 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -591,13 +591,14 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly)
|
||||
if (operation == CMD_SELECT && parseTree->into != NULL)
|
||||
{
|
||||
do_select_into = true;
|
||||
estate->es_select_into = true;
|
||||
|
||||
/*
|
||||
* For now, always create OIDs in SELECT INTO; this is for
|
||||
* backwards compatibility with pre-7.3 behavior. Eventually we
|
||||
* might want to allow the user to choose.
|
||||
*/
|
||||
estate->es_force_oids = true;
|
||||
estate->es_into_oids = true;
|
||||
}
|
||||
|
||||
/*
|
||||
@ -903,6 +904,63 @@ initResultRelInfo(ResultRelInfo *resultRelInfo,
|
||||
ExecOpenIndices(resultRelInfo);
|
||||
}
|
||||
|
||||
/*
|
||||
* ExecContextForcesOids
|
||||
*
|
||||
* This is pretty grotty: when doing INSERT, UPDATE, or SELECT INTO,
|
||||
* we need to ensure that result tuples have space for an OID iff they are
|
||||
* going to be stored into a relation that has OIDs. In other contexts
|
||||
* we are free to choose whether to leave space for OIDs in result tuples
|
||||
* (we generally don't want to, but we do if a physical-tlist optimization
|
||||
* is possible). This routine checks the plan context and returns TRUE if the
|
||||
* choice is forced, FALSE if the choice is not forced. In the TRUE case,
|
||||
* *hasoids is set to the required value.
|
||||
*
|
||||
* One reason this is ugly is that all plan nodes in the plan tree will emit
|
||||
* tuples with space for an OID, though we really only need the topmost node
|
||||
* to do so. However, node types like Sort don't project new tuples but just
|
||||
* return their inputs, and in those cases the requirement propagates down
|
||||
* to the input node. Eventually we might make this code smart enough to
|
||||
* recognize how far down the requirement really goes, but for now we just
|
||||
* make all plan nodes do the same thing if the top level forces the choice.
|
||||
*
|
||||
* We assume that estate->es_result_relation_info is already set up to
|
||||
* describe the target relation. Note that in an UPDATE that spans an
|
||||
* inheritance tree, some of the target relations may have OIDs and some not.
|
||||
* We have to make the decisions on a per-relation basis as we initialize
|
||||
* each of the child plans of the topmost Append plan.
|
||||
*
|
||||
* SELECT INTO is even uglier, because we don't have the INTO relation's
|
||||
* descriptor available when this code runs; we have to look aside at a
|
||||
* flag set by InitPlan().
|
||||
*/
|
||||
bool
|
||||
ExecContextForcesOids(PlanState *planstate, bool *hasoids)
|
||||
{
|
||||
if (planstate->state->es_select_into)
|
||||
{
|
||||
*hasoids = planstate->state->es_into_oids;
|
||||
return true;
|
||||
}
|
||||
else
|
||||
{
|
||||
ResultRelInfo *ri = planstate->state->es_result_relation_info;
|
||||
|
||||
if (ri != NULL)
|
||||
{
|
||||
Relation rel = ri->ri_RelationDesc;
|
||||
|
||||
if (rel != NULL)
|
||||
{
|
||||
*hasoids = rel->rd_rel->relhasoids;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/* ----------------------------------------------------------------
|
||||
* ExecEndPlan
|
||||
*
|
||||
@ -2063,7 +2121,8 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
|
||||
palloc0(estate->es_topPlan->nParamExec * sizeof(ParamExecData));
|
||||
epqstate->es_rowMark = estate->es_rowMark;
|
||||
epqstate->es_instrument = estate->es_instrument;
|
||||
epqstate->es_force_oids = estate->es_force_oids;
|
||||
epqstate->es_select_into = estate->es_select_into;
|
||||
epqstate->es_into_oids = estate->es_into_oids;
|
||||
epqstate->es_topPlan = estate->es_topPlan;
|
||||
|
||||
/*
|
||||
|
||||
@ -12,7 +12,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execScan.c,v 1.28 2003/09/25 19:41:49 tgl Exp $
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execScan.c,v 1.28.2.1 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -23,7 +23,7 @@
|
||||
#include "utils/memutils.h"
|
||||
|
||||
|
||||
static bool tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc);
|
||||
static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);
|
||||
|
||||
|
||||
/* ----------------------------------------------------------------
|
||||
@ -180,7 +180,8 @@ ExecAssignScanProjectionInfo(ScanState *node)
|
||||
{
|
||||
Scan *scan = (Scan *) node->ps.plan;
|
||||
|
||||
if (tlist_matches_tupdesc(scan->plan.targetlist,
|
||||
if (tlist_matches_tupdesc(&node->ps,
|
||||
scan->plan.targetlist,
|
||||
scan->scanrelid,
|
||||
node->ss_ScanTupleSlot->ttc_tupleDescriptor))
|
||||
node->ps.ps_ProjInfo = NULL;
|
||||
@ -189,11 +190,13 @@ ExecAssignScanProjectionInfo(ScanState *node)
|
||||
}
|
||||
|
||||
static bool
|
||||
tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc)
|
||||
tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
|
||||
{
|
||||
int numattrs = tupdesc->natts;
|
||||
int attrno;
|
||||
bool hasoid;
|
||||
|
||||
/* Check the tlist attributes */
|
||||
for (attrno = 1; attrno <= numattrs; attrno++)
|
||||
{
|
||||
Form_pg_attribute att_tup = tupdesc->attrs[attrno - 1];
|
||||
@ -219,5 +222,13 @@ tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc)
|
||||
if (tlist)
|
||||
return false; /* tlist too long */
|
||||
|
||||
/*
|
||||
* If the plan context requires a particular hasoid setting, then
|
||||
* that has to match, too.
|
||||
*/
|
||||
if (ExecContextForcesOids(ps, &hasoid) &&
|
||||
hasoid != tupdesc->tdhasoid)
|
||||
return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@ -8,7 +8,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.106.2.1 2003/12/18 20:21:53 tgl Exp $
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.106.2.2 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -201,7 +201,8 @@ CreateExecutorState(void)
|
||||
estate->es_rowMark = NIL;
|
||||
|
||||
estate->es_instrument = false;
|
||||
estate->es_force_oids = false;
|
||||
estate->es_select_into = false;
|
||||
estate->es_into_oids = false;
|
||||
|
||||
estate->es_exprcontexts = NIL;
|
||||
|
||||
@ -446,43 +447,17 @@ ExecAssignResultTypeFromOuterPlan(PlanState *planstate)
|
||||
void
|
||||
ExecAssignResultTypeFromTL(PlanState *planstate)
|
||||
{
|
||||
bool hasoid = false;
|
||||
bool hasoid;
|
||||
TupleDesc tupDesc;
|
||||
|
||||
/*
|
||||
* This is pretty grotty: we need to ensure that result tuples have
|
||||
* space for an OID iff they are going to be stored into a relation
|
||||
* that has OIDs. We assume that estate->es_result_relation_info is
|
||||
* already set up to describe the target relation. One reason this is
|
||||
* ugly is that all plan nodes in the plan tree will emit tuples with
|
||||
* space for an OID, though we really only need the topmost plan to do
|
||||
* so.
|
||||
*
|
||||
* It would be better to have InitPlan adjust the topmost plan node's
|
||||
* output descriptor after plan tree initialization. However, that
|
||||
* doesn't quite work because in an UPDATE that spans an inheritance
|
||||
* tree, some of the target relations may have OIDs and some not. We
|
||||
* have to make the decision on a per-relation basis as we initialize
|
||||
* each of the child plans of the topmost Append plan. So, this is
|
||||
* ugly but it works, for now ...
|
||||
*
|
||||
* SELECT INTO is also pretty grotty, because we don't yet have the INTO
|
||||
* relation's descriptor at this point; we have to look aside at a
|
||||
* flag set by InitPlan().
|
||||
*/
|
||||
if (planstate->state->es_force_oids)
|
||||
hasoid = true;
|
||||
if (ExecContextForcesOids(planstate, &hasoid))
|
||||
{
|
||||
/* context forces OID choice; hasoid is now set correctly */
|
||||
}
|
||||
else
|
||||
{
|
||||
ResultRelInfo *ri = planstate->state->es_result_relation_info;
|
||||
|
||||
if (ri != NULL)
|
||||
{
|
||||
Relation rel = ri->ri_RelationDesc;
|
||||
|
||||
if (rel != NULL)
|
||||
hasoid = rel->rd_rel->relhasoids;
|
||||
}
|
||||
/* given free choice, don't leave space for OIDs in result tuples */
|
||||
hasoid = false;
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@ -8,7 +8,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.54 2003/08/04 02:39:59 momjian Exp $
|
||||
* $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.54.4.1 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -208,7 +208,7 @@ ExecInitAppend(Append *node, EState *estate)
|
||||
* call ExecInitNode on each of the plans to be executed and save the
|
||||
* results into the array "appendplans". Note we *must* set
|
||||
* estate->es_result_relation_info correctly while we initialize each
|
||||
* sub-plan; ExecAssignResultTypeFromTL depends on that!
|
||||
* sub-plan; ExecContextForcesOids depends on that!
|
||||
*/
|
||||
for (i = appendstate->as_firstplan; i <= appendstate->as_lastplan; i++)
|
||||
{
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $Id: executor.h,v 1.102.2.1 2003/12/18 20:21:53 tgl Exp $
|
||||
* $Id: executor.h,v 1.102.2.2 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -93,6 +93,7 @@ extern void ExecutorEnd(QueryDesc *queryDesc);
|
||||
extern void ExecutorRewind(QueryDesc *queryDesc);
|
||||
extern void ExecCheckRTPerms(List *rangeTable, CmdType operation);
|
||||
extern void ExecEndPlan(PlanState *planstate, EState *estate);
|
||||
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
|
||||
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
|
||||
TupleTableSlot *slot, EState *estate);
|
||||
extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $Id: execnodes.h,v 1.107.2.1 2003/12/18 22:23:55 tgl Exp $
|
||||
* $Id: execnodes.h,v 1.107.2.2 2004/01/22 02:23:35 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -311,9 +311,8 @@ typedef struct EState
|
||||
List *es_rowMark; /* not good place, but there is no other */
|
||||
|
||||
bool es_instrument; /* true requests runtime instrumentation */
|
||||
bool es_force_oids; /* true forces result tuples to have
|
||||
* (space for) OIDs --- used for SELECT
|
||||
* INTO */
|
||||
bool es_select_into; /* true if doing SELECT INTO */
|
||||
bool es_into_oids; /* true to generate OIDs in SELECT INTO */
|
||||
|
||||
List *es_exprcontexts; /* List of ExprContexts within EState */
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user