I wrote:
> I'm having some difficulty choosing what to do refactoring-wise.
> ...
> If this were a HEAD-only patch I'd be tempted to do like Alvaro just did
> and move all the PARAM_EXEC assignment logic and root->plan_params
> and root->curOuterParams manipulation into a new file, say
> optimizer/util/paramassign.c. But that would be a little invasive
> for a back-patch.
Actually, maybe it wouldn't be that bad. The functions I'd want to
move into a new file are currently mostly static in subselect.c.
Moving them elsewhere (and consequently making them not-static)
would therefore not break any existing API. The exceptions are:
SS_assign_special_param
We could keep a wrapper by this name in the back branches, and thereby
avoid any API/ABI break for extension code using it (which it seems
somewhat likely that there might be).
SS_make_initplan_output_param
Although this'd just be a one-line wrapper for functionality exported
by the hypothetical new file, it's somewhat closely related to
SS_make_initplan_from_plan. So keeping it, with its current name,
isn't outlandish.
assign_nestloop_param_var
assign_nestloop_param_placeholdervar
These are going to go away, or else change API substantially, in any
case. Hopefully there's no extension code using them.
So what I'm now contemplating is moving these existing functions
to a new file:
int assign_param_for_var(PlannerInfo *root, Var *var)
Param *replace_outer_var(PlannerInfo *root, Var *var)
int assign_param_for_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
Param *replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
Param *replace_outer_agg(PlannerInfo *root, Aggref *agg)
Param *replace_outer_grouping(PlannerInfo *root, GroupingFunc *grp)
Param *generate_new_param(PlannerInfo *root, Oid paramtype, int32 paramtypmod,
Oid paramcollation)
int assign_special_param(PlannerInfo *root)
(rename of SS_assign_special_param)
along with some new functions for nestloop parameters (extracted from
createplan.c logic).
assign_param_for_var and assign_param_for_placeholdervar could remain
"static" since they aren't called from anywhere else. (Actually I
guess they'd end up getting merged back into the replace_outer_
functions, since they'll now have just one caller apiece.)
A variant idea would involve also moving replace_correlation_vars_mutator,
which would allow the individual replace_outer_xxx functions to remain
static. A different idea is to just move the assign_param_for_xxx
functionality, which'd require breaking replace_outer_agg and
replace_outer_grouping down into two functions apiece, since they
currently combine creation of plan_params items with creation of
the referencing Param nodes.
Anybody have a preference?
regards, tom lane
