Re: WIP: Upper planner pathification

Hi Tom,

On 2016-02-28 15:03:28 -0500, Tom Lane wrote:
> diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
> index eaa642b..cd7338a 100644
> *** a/src/include/optimizer/planmain.h
> --- b/src/include/optimizer/planmain.h
> *************** extern RelOptInfo *query_planner(Planner
> *** 43,102 ****
>    * prototypes for plan/planagg.c
>    */
>   extern void preprocess_minmax_aggregates(PlannerInfo *root, List *tlist);
> - extern Plan *optimize_minmax_aggregates(PlannerInfo *root, List *tlist,
> -                            const AggClauseCosts *aggcosts, Path *best_path);
>   
>   /*
>    * prototypes for plan/createplan.c
>    */
>   extern Plan *create_plan(PlannerInfo *root, Path *best_path);
> - extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
> -                   Index scanrelid, Plan *subplan);
>   extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
>                    Index scanrelid, List *fdw_exprs, List *fdw_private,
>                    List *fdw_scan_tlist, List *fdw_recheck_quals,
>                    Plan *outer_plan);
> - extern Append *make_append(List *appendplans, List *tlist);
> - extern RecursiveUnion *make_recursive_union(List *tlist,
> -                      Plan *lefttree, Plan *righttree, int wtParam,
> -                      List *distinctList, long numGroups);
> - extern Sort *make_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree,
> -                         List *pathkeys, double limit_tuples);
> - extern Sort *make_sort_from_sortclauses(PlannerInfo *root, List *sortcls,
> -                            Plan *lefttree);
> - extern Sort *make_sort_from_groupcols(PlannerInfo *root, List *groupcls,
> -                          AttrNumber *grpColIdx, Plan *lefttree);
> - extern Agg *make_agg(PlannerInfo *root, List *tlist, List *qual,
> -          AggStrategy aggstrategy, const AggClauseCosts *aggcosts,
> -          int numGroupCols, AttrNumber *grpColIdx, Oid *grpOperators,
> -          List *groupingSets, long numGroups, bool combineStates,
> -          bool finalizeAggs, Plan *lefttree);
> - extern WindowAgg *make_windowagg(PlannerInfo *root, List *tlist,
> -                List *windowFuncs, Index winref,
> -                int partNumCols, AttrNumber *partColIdx, Oid *partOperators,
> -                int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators,
> -                int frameOptions, Node *startOffset, Node *endOffset,
> -                Plan *lefttree);
> - extern Group *make_group(PlannerInfo *root, List *tlist, List *qual,
> -            int numGroupCols, AttrNumber *grpColIdx, Oid *grpOperators,
> -            double numGroups,
> -            Plan *lefttree);
>   extern Plan *materialize_finished_plan(Plan *subplan);
> ! extern Unique *make_unique(Plan *lefttree, List *distinctList);
> ! extern LockRows *make_lockrows(Plan *lefttree, List *rowMarks, int epqParam);
> ! extern Limit *make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount,
> !            int64 offset_est, int64 count_est);
> ! extern SetOp *make_setop(SetOpCmd cmd, SetOpStrategy strategy, Plan *lefttree,
> !            List *distinctList, AttrNumber flagColIdx, int firstFlag,
> !            long numGroups, double outputRows);
> ! extern Result *make_result(PlannerInfo *root, List *tlist,
> !             Node *resconstantqual, Plan *subplan);
> ! extern ModifyTable *make_modifytable(PlannerInfo *root,
> !                  CmdType operation, bool canSetTag,
> !                  Index nominalRelation,
> !                  List *resultRelations, List *subplans,
> !                  List *withCheckOptionLists, List *returningLists,
> !                  List *rowMarks, OnConflictExpr *onconflict, int epqParam);
>   extern bool is_projection_capable_plan(Plan *plan);


I see that you made a lot of formerly externally visible make_* routines
static. The Citus extension (which will be open source in a few days)
uses several of these (make_agg, make_sort_from_sortclauses, make_limit
at least).

Can we please re-consider making these static?

It's fine if their parameter list changes from release to release,
that's easy enough to adjust to, and it's easily visible. But having to
copy all of these into extension code is more than a bit painful;
especially make_sort_from_sortclauses.

Greetings,

Andres Freund