Skip to content

Commit 16f1cfc

Browse files
tglsfdctglsfdc
committed
Back-patch fix for bogus plans involving non-mark/restorable plan
as inner plan of a mergejoin.
1 parent 4fc8465 commit 16f1cfc

File tree

4 files changed

+69
-33
lines changed

4 files changed

+69
-33
lines changed

‎src/backend/executor/execAmi.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Id: execAmi.c,v 1.46 2000/04/12 17:15:07 momjian Exp $
9+
* $Id: execAmi.c,v 1.46.2.1 2000/09/08 02:11:32 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -424,14 +424,18 @@ ExecMarkPos(Plan *node)
424424
{
425425
switch (nodeTag(node))
426426
{
427-
case T_SeqScan:
427+
case T_SeqScan:
428428
ExecSeqMarkPos((SeqScan *) node);
429429
break;
430430

431431
case T_IndexScan:
432432
ExecIndexMarkPos((IndexScan *) node);
433433
break;
434434

435+
case T_Material:
436+
ExecMaterialMarkPos((Material *) node);
437+
break;
438+
435439
case T_Sort:
436440
ExecSortMarkPos((Sort *) node);
437441
break;
@@ -458,20 +462,24 @@ ExecRestrPos(Plan *node)
458462
{
459463
switch (nodeTag(node))
460464
{
461-
case T_SeqScan:
465+
case T_SeqScan:
462466
ExecSeqRestrPos((SeqScan *) node);
463467
return;
464468

465469
case T_IndexScan:
466470
ExecIndexRestrPos((IndexScan *) node);
467471
return;
468472

473+
case T_Material:
474+
ExecMaterialRestrPos((Material *) node);
475+
break;
476+
469477
case T_Sort:
470478
ExecSortRestrPos((Sort *) node);
471479
return;
472480

473481
default:
474-
elog(DEBUG, "ExecRestrPos: node type %u not supported", nodeTag(node));
482+
elog(ERROR, "ExecRestrPos: node type %u not supported", nodeTag(node));
475483
return;
476484
}
477485
}

‎src/backend/executor/nodeMaterial.c

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/executor/nodeMaterial.c,v 1.30 2000/03/02 04:06:39 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/executor/nodeMaterial.c,v 1.30.2.1 2000/09/08 02:11:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -352,61 +352,54 @@ ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent)
352352

353353
}
354354

355-
#ifdef NOT_USED /* not used */
356355
/* ----------------------------------------------------------------
357356
* ExecMaterialMarkPos
358357
* ----------------------------------------------------------------
359358
*/
360-
List /* nothing of interest */
361-
ExecMaterialMarkPos(Material node)
359+
void
360+
ExecMaterialMarkPos(Material *node)
362361
{
363-
MaterialState matstate;
362+
MaterialState *matstate;
364363
HeapScanDesc scan;
365364

366365
/* ----------------
367-
* if we haven't materialized yet, just return NIL.
366+
* if we haven't materialized yet, just return.
368367
* ----------------
369368
*/
370-
matstate = get_matstate(node);
371-
if (get_mat_Flag(matstate) == false)
372-
return NIL;
369+
matstate = node->matstate;
370+
if (matstate->mat_Flag == false)
371+
return;
373372

374373
/* ----------------
375-
* XXX access methods don't return positions yet so
376-
* for now we return NIL. It's possible that
377-
* they will never return positions for all I know -cim 10/16/89
374+
* mark the scan position
378375
* ----------------
379376
*/
380-
scan = get_css_currentScanDesc((CommonScanState) matstate);
377+
scan = matstate->csstate.css_currentScanDesc;
381378
heap_markpos(scan);
382-
383-
return NIL;
384379
}
385380

386381
/* ----------------------------------------------------------------
387382
* ExecMaterialRestrPos
388383
* ----------------------------------------------------------------
389384
*/
390385
void
391-
ExecMaterialRestrPos(Material node)
386+
ExecMaterialRestrPos(Material *node)
392387
{
393-
MaterialState matstate;
388+
MaterialState *matstate;
394389
HeapScanDesc scan;
395390

396391
/* ----------------
397392
* if we haven't materialized yet, just return.
398393
* ----------------
399394
*/
400-
matstate = get_matstate(node);
401-
if (get_mat_Flag(matstate) == false)
395+
matstate = node->matstate;
396+
if (matstate->mat_Flag == false)
402397
return;
403398

404399
/* ----------------
405400
* restore the scan to the previously marked position
406401
* ----------------
407402
*/
408-
scan = get_css_currentScanDesc((CommonScanState) matstate);
403+
scan = matstate->csstate.css_currentScanDesc;
409404
heap_restrpos(scan);
410405
}
411-
412-
#endif

‎src/backend/optimizer/plan/createplan.c

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.90 2000/05/23 16:56:36 tgl Exp $
13+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.90.2.1 2000/09/08 02:11:33 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -639,6 +639,44 @@ create_mergejoin_node(MergePath *best_path,
639639
best_path->innersortkeys,
640640
inner_node);
641641

642+
/*
643+
* The executor requires the inner side of a mergejoin to support "mark"
644+
* and "restore" operations. Not all plan types do, so we must be careful
645+
* not to generate an invalid plan. If necessary, an invalid inner plan
646+
* can be handled by inserting a Materialize node.
647+
*
648+
* Since the inner side must be ordered, and only Sorts and IndexScans can
649+
* create order to begin with, you might think there's no problem --- but
650+
* you'd be wrong. Nestloop and merge joins can *preserve* the order of
651+
* their inputs, so they can be selected as the input of a mergejoin,
652+
* and that won't work in the present executor.
653+
*
654+
* Doing this here is a bit of a kluge since the cost of the Materialize
655+
* wasn't taken into account in our earlier decisions. But Materialize
656+
* is hard to estimate a cost for, and the above consideration shows that
657+
* this is a rare case anyway, so this seems an acceptable way to proceed.
658+
*
659+
* This check must agree with ExecMarkPos/ExecRestrPos in
660+
* executor/execAmi.c!
661+
*/
662+
switch (nodeTag(inner_node))
663+
{
664+
case T_SeqScan:
665+
case T_IndexScan:
666+
case T_Material:
667+
case T_Sort:
668+
/* OK, these inner plans support mark/restore */
669+
break;
670+
671+
default:
672+
/* Ooops, need to materialize the inner plan */
673+
inner_node = (Plan *) make_material(inner_tlist,
674+
_NONAME_RELATION_ID_,
675+
inner_node,
676+
0);
677+
break;
678+
}
679+
642680
join_node = make_mergejoin(tlist,
643681
qpqual,
644682
mergeclauses,

‎src/include/executor/nodeMaterial.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: nodeMaterial.h,v 1.12 2000/01/26 05:58:05 momjian Exp $
10+
* $Id: nodeMaterial.h,v 1.12.2.1 2000/09/08 02:11:31 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -20,11 +20,8 @@ extern TupleTableSlot *ExecMaterial(Material *node);
2020
extern bool ExecInitMaterial(Material *node, EState *estate, Plan *parent);
2121
extern int ExecCountSlotsMaterial(Material *node);
2222
extern void ExecEndMaterial(Material *node);
23-
extern void ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent);
24-
25-
#ifdef NOT_USED
26-
extern List ExecMaterialMarkPos(Material *node);
23+
extern void ExecMaterialMarkPos(Material *node);
2724
extern void ExecMaterialRestrPos(Material *node);
25+
extern void ExecMaterialReScan(Material *node, ExprContext *exprCtxt, Plan *parent);
2826

29-
#endif
3027
#endif /* NODEMATERIAL_H */

0 commit comments

Comments
 (0)