From 16f1cfcb87584336641629180b4e8dc8cf9f304f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 8 Sep 2000 02:11:33 +0000 Subject: [PATCH] Back-patch fix for bogus plans involving non-mark/restorable plan as inner plan of a mergejoin. --- src/backend/executor/execAmi.c | 16 +++++++--- src/backend/executor/nodeMaterial.c | 37 ++++++++++------------- src/backend/optimizer/plan/createplan.c | 40 ++++++++++++++++++++++++- src/include/executor/nodeMaterial.h | 9 ++---- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c index 579465bf0b8..376eeef8b5d 100644 --- a/src/backend/executor/execAmi.c +++ b/src/backend/executor/execAmi.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: execAmi.c,v 1.46 2000/04/12 17:15:07 momjian Exp $ + * $Id: execAmi.c,v 1.46.2.1 2000/09/08 02:11:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -424,7 +424,7 @@ ExecMarkPos(Plan *node) { switch (nodeTag(node)) { - case T_SeqScan: + case T_SeqScan: ExecSeqMarkPos((SeqScan *) node); break; @@ -432,6 +432,10 @@ ExecMarkPos(Plan *node) ExecIndexMarkPos((IndexScan *) node); break; + case T_Material: + ExecMaterialMarkPos((Material *) node); + break; + case T_Sort: ExecSortMarkPos((Sort *) node); break; @@ -458,7 +462,7 @@ ExecRestrPos(Plan *node) { switch (nodeTag(node)) { - case T_SeqScan: + case T_SeqScan: ExecSeqRestrPos((SeqScan *) node); return; @@ -466,12 +470,16 @@ ExecRestrPos(Plan *node) ExecIndexRestrPos((IndexScan *) node); return; + case T_Material: + ExecMaterialRestrPos((Material *) node); + break; + case T_Sort: ExecSortRestrPos((Sort *) node); return; default: - elog(DEBUG, "ExecRestrPos: node type %u not supported", nodeTag(node)); + elog(ERROR, "ExecRestrPos: node type %u not supported", nodeTag(node)); return; } } diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c index 4348f89ccc9..4f67892682b 100644 --- a/src/backend/executor/nodeMaterial.c +++ b/src/backend/executor/nodeMaterial.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/nodeMaterial.c,v 1.30 2000/03/02 04:06:39 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/nodeMaterial.c,v 1.30.2.1 2000/09/08 02:11:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -352,35 +352,30 @@ ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent) } -#ifdef NOT_USED /* not used */ /* ---------------------------------------------------------------- * ExecMaterialMarkPos * ---------------------------------------------------------------- */ -List /* nothing of interest */ -ExecMaterialMarkPos(Material node) +void +ExecMaterialMarkPos(Material *node) { - MaterialState matstate; + MaterialState *matstate; HeapScanDesc scan; /* ---------------- - * if we haven't materialized yet, just return NIL. + * if we haven't materialized yet, just return. * ---------------- */ - matstate = get_matstate(node); - if (get_mat_Flag(matstate) == false) - return NIL; + matstate = node->matstate; + if (matstate->mat_Flag == false) + return; /* ---------------- - * XXX access methods don't return positions yet so - * for now we return NIL. It's possible that - * they will never return positions for all I know -cim 10/16/89 + * mark the scan position * ---------------- */ - scan = get_css_currentScanDesc((CommonScanState) matstate); + scan = matstate->csstate.css_currentScanDesc; heap_markpos(scan); - - return NIL; } /* ---------------------------------------------------------------- @@ -388,25 +383,23 @@ ExecMaterialMarkPos(Material node) * ---------------------------------------------------------------- */ void -ExecMaterialRestrPos(Material node) +ExecMaterialRestrPos(Material *node) { - MaterialState matstate; + MaterialState *matstate; HeapScanDesc scan; /* ---------------- * if we haven't materialized yet, just return. * ---------------- */ - matstate = get_matstate(node); - if (get_mat_Flag(matstate) == false) + matstate = node->matstate; + if (matstate->mat_Flag == false) return; /* ---------------- * restore the scan to the previously marked position * ---------------- */ - scan = get_css_currentScanDesc((CommonScanState) matstate); + scan = matstate->csstate.css_currentScanDesc; heap_restrpos(scan); } - -#endif diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 4109ab25364..7d4a34b4f4b 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.90 2000/05/23 16:56:36 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.90.2.1 2000/09/08 02:11:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -639,6 +639,44 @@ create_mergejoin_node(MergePath *best_path, best_path->innersortkeys, inner_node); + /* + * The executor requires the inner side of a mergejoin to support "mark" + * and "restore" operations. Not all plan types do, so we must be careful + * not to generate an invalid plan. If necessary, an invalid inner plan + * can be handled by inserting a Materialize node. + * + * Since the inner side must be ordered, and only Sorts and IndexScans can + * create order to begin with, you might think there's no problem --- but + * you'd be wrong. Nestloop and merge joins can *preserve* the order of + * their inputs, so they can be selected as the input of a mergejoin, + * and that won't work in the present executor. + * + * Doing this here is a bit of a kluge since the cost of the Materialize + * wasn't taken into account in our earlier decisions. But Materialize + * is hard to estimate a cost for, and the above consideration shows that + * this is a rare case anyway, so this seems an acceptable way to proceed. + * + * This check must agree with ExecMarkPos/ExecRestrPos in + * executor/execAmi.c! + */ + switch (nodeTag(inner_node)) + { + case T_SeqScan: + case T_IndexScan: + case T_Material: + case T_Sort: + /* OK, these inner plans support mark/restore */ + break; + + default: + /* Ooops, need to materialize the inner plan */ + inner_node = (Plan *) make_material(inner_tlist, + _NONAME_RELATION_ID_, + inner_node, + 0); + break; + } + join_node = make_mergejoin(tlist, qpqual, mergeclauses, diff --git a/src/include/executor/nodeMaterial.h b/src/include/executor/nodeMaterial.h index 1daf0bd0afe..0696cba155a 100644 --- a/src/include/executor/nodeMaterial.h +++ b/src/include/executor/nodeMaterial.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: nodeMaterial.h,v 1.12 2000/01/26 05:58:05 momjian Exp $ + * $Id: nodeMaterial.h,v 1.12.2.1 2000/09/08 02:11:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -20,11 +20,8 @@ extern TupleTableSlot *ExecMaterial(Material *node); extern bool ExecInitMaterial(Material *node, EState *estate, Plan *parent); extern int ExecCountSlotsMaterial(Material *node); extern void ExecEndMaterial(Material *node); -extern void ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent); - -#ifdef NOT_USED -extern List ExecMaterialMarkPos(Material *node); +extern void ExecMaterialMarkPos(Material *node); extern void ExecMaterialRestrPos(Material *node); +extern void ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent); -#endif #endif /* NODEMATERIAL_H */ -- 2.39.5