Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commita34fa8e

Browse files
committed
Initial code review for CustomScan patch.
Get rid of the pernicious entanglement between planner and executor headersintroduced by commit0b03e59.Also, rearrange the CustomFoo struct/typedef definitions so that all thetypedef names are seen as used by the compiler. Without this pgindentwill mess things up a bit, which is not so important perhaps, but it alsoremoves a bizarre discrepancy between the declaration arrangement used forCustomExecMethods and that used for CustomScanMethods andCustomPathMethods.Clean up the commentary around ExecSupportsMarkRestore to reflect therather large change in its API.Const-ify register_custom_path_provider's argument. This necessitatescasting away const in the function, but that seems better than forcingcallers of the function to do so (or else not const-ify their methodpointer structs, which was sort of the whole point).De-export fix_expr_common. I don't like the exporting of fix_scan_expror replace_nestloop_params either, but this one surely has got littleexcuse.
1 parent081a604 commita34fa8e

File tree

11 files changed

+164
-156
lines changed

11 files changed

+164
-156
lines changed

‎src/backend/executor/execAmi.c‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -381,11 +381,14 @@ ExecRestrPos(PlanState *node)
381381
}
382382

383383
/*
384-
* ExecSupportsMarkRestore - does a plan type support mark/restore?
384+
* ExecSupportsMarkRestore - does a Path support mark/restore?
385+
*
386+
* This is used during planning and so must accept a Path, not a Plan.
387+
* We keep it here to be adjacent to the routines above, which also must
388+
* know which plan types support mark/restore.
385389
*
386390
* XXX Ideally, all plan node types would support mark/restore, and this
387391
* wouldn't be needed. For now, this had better match the routines above.
388-
* But note the test is on Plan nodetype, not PlanState nodetype.
389392
*
390393
* (However, since the only present use of mark/restore is in mergejoin,
391394
* there is no need to support mark/restore in any plan type that is not
@@ -395,6 +398,11 @@ ExecRestrPos(PlanState *node)
395398
bool
396399
ExecSupportsMarkRestore(Path*pathnode)
397400
{
401+
/*
402+
* For consistency with the routines above, we do not examine the nodeTag
403+
* but rather the pathtype, which is the Plan node type the Path would
404+
* produce.
405+
*/
398406
switch (pathnode->pathtype)
399407
{
400408
caseT_SeqScan:
@@ -406,27 +414,23 @@ ExecSupportsMarkRestore(Path *pathnode)
406414
caseT_Sort:
407415
return true;
408416

417+
caseT_CustomScan:
418+
Assert(IsA(pathnode,CustomPath));
419+
if (((CustomPath*)pathnode)->flags&CUSTOMPATH_SUPPORT_MARK_RESTORE)
420+
return true;
421+
return false;
422+
409423
caseT_Result:
410424

411425
/*
412-
* T_Result only supports mark/restore if it has a child plan that
413-
* does, so we do not have enough information to give a really
414-
* correct answer. However, for current uses it's enough to
415-
* always say "false", because this routine is not asked about
416-
* gating Result plans, only base-case Results.
426+
* Although Result supports mark/restore if it has a child plan
427+
* that does, we presently come here only for ResultPath nodes,
428+
* which represent Result plans without a child plan. So there is
429+
* nothing to recurse to and we can just say "false".
417430
*/
431+
Assert(IsA(pathnode,ResultPath));
418432
return false;
419433

420-
caseT_CustomScan:
421-
{
422-
CustomPath*cpath= (CustomPath*)pathnode;
423-
424-
Assert(IsA(cpath,CustomPath));
425-
if (cpath->flags&CUSTOMPATH_SUPPORT_MARK_RESTORE)
426-
return true;
427-
}
428-
break;
429-
430434
default:
431435
break;
432436
}
@@ -491,10 +495,10 @@ ExecSupportsBackwardScan(Plan *node)
491495

492496
caseT_CustomScan:
493497
{
494-
uint32flags= ((CustomScan*)node)->flags;
498+
uint32flags= ((CustomScan*)node)->flags;
495499

496-
if (TargetListSupportsBackwardScan(node->targetlist)&&
497-
(flags&CUSTOMPATH_SUPPORT_BACKWARD_SCAN)!=0)
500+
if ((flags&CUSTOMPATH_SUPPORT_BACKWARD_SCAN)&&
501+
TargetListSupportsBackwardScan(node->targetlist))
498502
return true;
499503
}
500504
return false;

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

Lines changed: 49 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ static WorkTableScan *create_worktablescan_plan(PlannerInfo *root, Path *best_pa
7878
staticForeignScan*create_foreignscan_plan(PlannerInfo*root,ForeignPath*best_path,
7979
List*tlist,List*scan_clauses);
8080
staticPlan*create_customscan_plan(PlannerInfo*root,
81-
CustomPath*best_path,
82-
List*tlist,List*scan_clauses);
81+
CustomPath*best_path,
82+
List*tlist,List*scan_clauses);
8383
staticNestLoop*create_nestloop_plan(PlannerInfo*root,NestPath*best_path,
8484
Plan*outer_plan,Plan*inner_plan);
8585
staticMergeJoin*create_mergejoin_plan(PlannerInfo*root,MergePath*best_path,
@@ -1083,52 +1083,6 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path)
10831083
returnplan;
10841084
}
10851085

1086-
/*
1087-
* create_custom_plan
1088-
*
1089-
* Transform a CustomPath into a Plan.
1090-
*/
1091-
staticPlan*
1092-
create_customscan_plan(PlannerInfo*root,CustomPath*best_path,
1093-
List*tlist,List*scan_clauses)
1094-
{
1095-
Plan*plan;
1096-
RelOptInfo*rel=best_path->path.parent;
1097-
1098-
/*
1099-
* Right now, all we can support is CustomScan node which is associated
1100-
* with a particular base relation to be scanned.
1101-
*/
1102-
Assert(rel&&rel->reloptkind==RELOPT_BASEREL);
1103-
1104-
/*
1105-
* Sort clauses into the best execution order, although custom-scan
1106-
* provider can reorder them again.
1107-
*/
1108-
scan_clauses=order_qual_clauses(root,scan_clauses);
1109-
1110-
/*
1111-
* Create a CustomScan (or its inheritance) node according to
1112-
* the supplied CustomPath.
1113-
*/
1114-
plan=best_path->methods->PlanCustomPath(root,rel,best_path,tlist,
1115-
scan_clauses);
1116-
1117-
/*
1118-
* NOTE: unlike create_foreignscan_plan(), it is responsibility of
1119-
* the custom plan provider to replace outer-relation variables
1120-
* with nestloop params, because we cannot know how many expression
1121-
* trees are held in the private fields.
1122-
*/
1123-
1124-
/*
1125-
* Copy cost data from Path to Plan; no need to make custom-plan
1126-
* providers do this
1127-
*/
1128-
copy_path_costsize(plan,&best_path->path);
1129-
1130-
returnplan;
1131-
}
11321086

11331087
/*****************************************************************************
11341088
*
@@ -2063,6 +2017,53 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
20632017
returnscan_plan;
20642018
}
20652019

2020+
/*
2021+
* create_custom_plan
2022+
*
2023+
* Transform a CustomPath into a Plan.
2024+
*/
2025+
staticPlan*
2026+
create_customscan_plan(PlannerInfo*root,CustomPath*best_path,
2027+
List*tlist,List*scan_clauses)
2028+
{
2029+
Plan*plan;
2030+
RelOptInfo*rel=best_path->path.parent;
2031+
2032+
/*
2033+
* Right now, all we can support is CustomScan node which is associated
2034+
* with a particular base relation to be scanned.
2035+
*/
2036+
Assert(rel&&rel->reloptkind==RELOPT_BASEREL);
2037+
2038+
/*
2039+
* Sort clauses into the best execution order, although custom-scan
2040+
* provider can reorder them again.
2041+
*/
2042+
scan_clauses=order_qual_clauses(root,scan_clauses);
2043+
2044+
/*
2045+
* Invoke custom plan provider to create the Plan node represented by the
2046+
* CustomPath.
2047+
*/
2048+
plan=best_path->methods->PlanCustomPath(root,rel,best_path,tlist,
2049+
scan_clauses);
2050+
2051+
/*
2052+
* NOTE: unlike create_foreignscan_plan(), it is the responsibility of the
2053+
* custom plan provider to replace outer-relation variables with nestloop
2054+
* params, because we cannot know what expression trees may be held in
2055+
* private fields.
2056+
*/
2057+
2058+
/*
2059+
* Copy cost data from Path to Plan; no need to make custom-plan providers
2060+
* do this
2061+
*/
2062+
copy_path_costsize(plan,&best_path->path);
2063+
2064+
returnplan;
2065+
}
2066+
20662067

20672068
/*****************************************************************************
20682069
*

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -587,12 +587,13 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
587587
fix_scan_list(root,cscan->scan.plan.targetlist,rtoffset);
588588
cscan->scan.plan.qual=
589589
fix_scan_list(root,cscan->scan.plan.qual,rtoffset);
590+
590591
/*
591-
* The core implementation applies the routine to fixup
592-
*varnoon the target-list and scan qualifier.
593-
*If custom-scan hasadditional expression nodes on its
594-
*private fields, it hasto apply same fixup on them.
595-
*Otherwise, the custom-planprovider can skip this callback.
592+
* The core implementation applies the routine to fixup varno
593+
* on the target-list and scan qualifier. If custom-scan has
594+
* additional expression nodes on its private fields, it has
595+
* to apply same fixup on them. Otherwise, the custom-plan
596+
* provider can skip this callback.
596597
*/
597598
if (cscan->methods->SetCustomScanRef)
598599
cscan->methods->SetCustomScanRef(root,cscan,rtoffset);
@@ -1083,7 +1084,7 @@ copyVar(Var *var)
10831084
* We assume it's okay to update opcode info in-place. So this could possibly
10841085
* scribble on the planner's input data structures, but it's OK.
10851086
*/
1086-
void
1087+
staticvoid
10871088
fix_expr_common(PlannerInfo*root,Node*node)
10881089
{
10891090
/* We assume callers won't call us on a NULL pointer */

‎src/backend/optimizer/util/pathnode.c‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,10 +1929,10 @@ reparameterize_path(PlannerInfo *root, Path *path,
19291929
}
19301930

19311931
/*****************************************************************************
1932-
* creation of custom-plan paths
1932+
* creation of custom-plan paths
19331933
*****************************************************************************/
19341934

1935-
staticList*custom_path_providers=NIL;
1935+
staticList*custom_path_providers=NIL;
19361936

19371937
/*
19381938
* register_custom_path_provider
@@ -1942,12 +1942,13 @@ static List *custom_path_providers = NIL;
19421942
* methods of scanning a relation.
19431943
*/
19441944
void
1945-
register_custom_path_provider(CustomPathMethods*cpp_methods)
1945+
register_custom_path_provider(constCustomPathMethods*cpp_methods)
19461946
{
1947-
MemoryContextoldcxt;
1947+
MemoryContextoldcxt;
19481948

19491949
oldcxt=MemoryContextSwitchTo(TopMemoryContext);
1950-
custom_path_providers=lappend(custom_path_providers,cpp_methods);
1950+
custom_path_providers=lappend(custom_path_providers,
1951+
(void*)cpp_methods);
19511952
MemoryContextSwitchTo(oldcxt);
19521953
}
19531954

@@ -1963,9 +1964,9 @@ create_customscan_paths(PlannerInfo *root,
19631964
RelOptInfo*baserel,
19641965
RangeTblEntry*rte)
19651966
{
1966-
ListCell*cell;
1967+
ListCell*cell;
19671968

1968-
foreach(cell,custom_path_providers)
1969+
foreach(cell,custom_path_providers)
19691970
{
19701971
constCustomPathMethods*cpp_methods=lfirst(cell);
19711972

‎src/include/executor/executor.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
#include"executor/execdesc.h"
1818
#include"nodes/parsenodes.h"
19-
#include"nodes/relation.h"
2019
#include"utils/lockwaitpolicy.h"
2120

2221

@@ -101,10 +100,12 @@ extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook;
101100
/*
102101
* prototypes from functions in execAmi.c
103102
*/
103+
structPath;/* avoid including relation.h here */
104+
104105
externvoidExecReScan(PlanState*node);
105106
externvoidExecMarkPos(PlanState*node);
106107
externvoidExecRestrPos(PlanState*node);
107-
externboolExecSupportsMarkRestore(Path*pathnode);
108+
externboolExecSupportsMarkRestore(structPath*pathnode);
108109
externboolExecSupportsBackwardScan(Plan*node);
109110
externboolExecMaterializesOutput(NodeTagplantype);
110111

‎src/include/executor/nodeCustom.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@
1111
*/
1212
#ifndefNODECUSTOM_H
1313
#defineNODECUSTOM_H
14-
#include"nodes/plannodes.h"
14+
1515
#include"nodes/execnodes.h"
1616

1717
/*
1818
* General executor code
1919
*/
2020
externCustomScanState*ExecInitCustomScan(CustomScan*custom_scan,
21-
EState*estate,inteflags);
21+
EState*estate,inteflags);
2222
externTupleTableSlot*ExecCustomScan(CustomScanState*node);
2323
externNode*MultiExecCustomScan(CustomScanState*node);
2424
externvoidExecEndCustomScan(CustomScanState*node);
@@ -27,4 +27,4 @@ extern void ExecReScanCustomScan(CustomScanState *node);
2727
externvoidExecCustomMarkPos(CustomScanState*node);
2828
externvoidExecCustomRestrPos(CustomScanState*node);
2929

30-
#endif/* NODECUSTOM_H */
30+
#endif/* NODECUSTOM_H */

‎src/include/nodes/execnodes.h‎

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include"executor/instrument.h"
2020
#include"nodes/params.h"
2121
#include"nodes/plannodes.h"
22-
#include"nodes/relation.h"
2322
#include"utils/reltrigger.h"
2423
#include"utils/sortsupport.h"
2524
#include"utils/tuplestore.h"
@@ -1512,39 +1511,39 @@ typedef struct ForeignScanState
15121511
*CustomScan nodes are used to execute custom code within executor.
15131512
* ----------------
15141513
*/
1515-
structCustomExecMethods;
1516-
structExplainState;/* to avoid to include explain.h here */
1517-
1518-
typedefstructCustomScanState
1519-
{
1520-
ScanStatess;
1521-
uint32flags;/* mask of CUSTOMPATH_* flags defined in relation.h*/
1522-
conststructCustomExecMethods*methods;
1523-
}CustomScanState;
1514+
structExplainState;/* avoid including explain.h here */
1515+
structCustomScanState;
15241516

15251517
typedefstructCustomExecMethods
15261518
{
1527-
constchar*CustomName;
1519+
constchar*CustomName;
15281520

15291521
/* EXECUTOR methods */
1530-
void(*BeginCustomScan)(CustomScanState*node,
1531-
EState*estate,
1532-
inteflags);
1533-
TupleTableSlot*(*ExecCustomScan)(CustomScanState*node);
1534-
void(*EndCustomScan)(CustomScanState*node);
1535-
void(*ReScanCustomScan)(CustomScanState*node);
1536-
void(*MarkPosCustomScan)(CustomScanState*node);
1537-
void(*RestrPosCustomScan)(CustomScanState*node);
1522+
void(*BeginCustomScan) (structCustomScanState*node,
1523+
EState*estate,
1524+
inteflags);
1525+
TupleTableSlot*(*ExecCustomScan) (structCustomScanState*node);
1526+
void(*EndCustomScan) (structCustomScanState*node);
1527+
void(*ReScanCustomScan) (structCustomScanState*node);
1528+
void(*MarkPosCustomScan) (structCustomScanState*node);
1529+
void(*RestrPosCustomScan) (structCustomScanState*node);
15381530

15391531
/* EXPLAIN support */
1540-
void(*ExplainCustomScan)(CustomScanState*node,
1541-
List*ancestors,
1542-
structExplainState*es);
1543-
Node*(*GetSpecialCustomVar)(CustomScanState*node,
1544-
Var*varnode,
1545-
PlanState**child_ps);
1532+
void(*ExplainCustomScan) (structCustomScanState*node,
1533+
List*ancestors,
1534+
structExplainState*es);
1535+
Node*(*GetSpecialCustomVar) (structCustomScanState*node,
1536+
Var*varnode,
1537+
PlanState**child_ps);
15461538
}CustomExecMethods;
15471539

1540+
typedefstructCustomScanState
1541+
{
1542+
ScanStatess;
1543+
uint32flags;/* mask of CUSTOMPATH_* flags, see relation.h */
1544+
constCustomExecMethods*methods;
1545+
}CustomScanState;
1546+
15481547
/* ----------------------------------------------------------------
15491548
* Join State Information
15501549
* ----------------------------------------------------------------

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp