Thread: Get rid of runtime handling of AlternativeSubPlan?
Back in bd3daddaf232d95b0c9ba6f99b0170a0147dd8af, which introduced AlternativeSubPlans, I wrote: There is a lot more that could be done based on this infrastructure: in particular it's interesting to consider switching to the hash plan if we start out using the non-hashed plan but find a lot more upper rows going by than we expected. I have therefore left some minor inefficiencies in place, such as initializing both subplans even though we will currently only use one. That commit will be twelve years old come August, and nobody has either built anything else atop it or shown any interest in making the plan choice switchable mid-run. So it seems like kind of a failed experiment. Therefore, I'm considering the idea of ripping out all executor support for AlternativeSubPlan and instead having the planner replace an AlternativeSubPlan with the desired specific SubPlan somewhere late in planning, possibly setrefs.c. Admittedly, the relevant executor support only amounts to a couple hundred lines, but that's not nothing. A perhaps-more-useful effect is to get rid of the confusing and poorly documented EXPLAIN output that you get for an AlternativeSubPlan. I also noted that the existing subplan-selection logic in ExecInitAlternativeSubPlan is really pretty darn bogus, in that it uses a one-size-fits-all execution count estimate of parent->plan->plan_rows, no matter which subexpression the subplan is in. This is only appropriate for subplans in the plan node's targetlist, and can be either too high or too low elsewhere. It'd be relatively easy for setrefs.c to do better, I think, since it knows which subexpression it's working on at any point. Thoughts? regards, tom lane
On Mon, 22 Jun 2020 at 12:20, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Back in bd3daddaf232d95b0c9ba6f99b0170a0147dd8af, which introduced > AlternativeSubPlans, I wrote: > > There is a lot more that could be done based on this infrastructure: in > particular it's interesting to consider switching to the hash plan if we start > out using the non-hashed plan but find a lot more upper rows going by than we > expected. I have therefore left some minor inefficiencies in place, such as > initializing both subplans even though we will currently only use one. > > That commit will be twelve years old come August, and nobody has either > built anything else atop it or shown any interest in making the plan choice > switchable mid-run. So it seems like kind of a failed experiment. > > Therefore, I'm considering the idea of ripping out all executor support > for AlternativeSubPlan and instead having the planner replace an > AlternativeSubPlan with the desired specific SubPlan somewhere late in > planning, possibly setrefs.c. > > Admittedly, the relevant executor support only amounts to a couple hundred > lines, but that's not nothing. A perhaps-more-useful effect is to get rid > of the confusing and poorly documented EXPLAIN output that you get for an > AlternativeSubPlan. > > I also noted that the existing subplan-selection logic in > ExecInitAlternativeSubPlan is really pretty darn bogus, in that it uses a > one-size-fits-all execution count estimate of parent->plan->plan_rows, no > matter which subexpression the subplan is in. This is only appropriate > for subplans in the plan node's targetlist, and can be either too high > or too low elsewhere. It'd be relatively easy for setrefs.c to do > better, I think, since it knows which subexpression it's working on > at any point. When I was working on [1] a few weeks ago, I did wonder if I'd have to use an AlternativeSubPlan when doing result caching for subqueries. The problem is likely the same as why they were invented in the first place; we basically don't know how many rows the parent will produce when planning the subplan. For my case, I have an interest in both the number of rows in the outer plan, and the ndistinct estimate on the subplan parameters. If the parameters for the subquery are all distinct, then there's not much sense in trying to cache results to use later. We're never going to need them. Right now, if I wanted to use AlternativeSubPlan to delay the choice of this until run-time, then I'd be missing information about the ndistinct estimation since we don't have that information available in the final plan. Perhaps that's an argument for doing this in setrefs.c instead. I could look up the ndistinct estimate there. For switching plans on the fly during execution. I can see the sense in that as an idea. For the hashed subplan case, we'd likely want to switch to hashing mode if we discovered that there were many more rows in the outer query than we had thought there would be. However, I'm uncertain if Result Cache would never need anything similar as technically we could just switch off the caching if we discovered our cache hit ration was either terrible or 0. We would have an additional node to pull tuples through, however. Switching would also require that the tupleslot type was the same between the alternatives. David [1] https://www.postgresql.org/message-id/CAApHDvrPcQyQdWERGYWx8J+2DLUNgXu+fOSbQ1UscxrunyXyrQ@mail.gmail.com
I wrote: > Back in bd3daddaf232d95b0c9ba6f99b0170a0147dd8af, which introduced > AlternativeSubPlans, I wrote: > There is a lot more that could be done based on this infrastructure: in > particular it's interesting to consider switching to the hash plan if we start > out using the non-hashed plan but find a lot more upper rows going by than we > expected. I have therefore left some minor inefficiencies in place, such as > initializing both subplans even though we will currently only use one. > > That commit will be twelve years old come August, and nobody has either > built anything else atop it or shown any interest in making the plan choice > switchable mid-run. So it seems like kind of a failed experiment. > > Therefore, I'm considering the idea of ripping out all executor support > for AlternativeSubPlan and instead having the planner replace an > AlternativeSubPlan with the desired specific SubPlan somewhere late in > planning, possibly setrefs.c. Here's a proposed patchset for that. This runs with the idea I'd had that setrefs.c could be smarter than the executor about which plan node subexpressions will be executed how many times. I did not take it very far, for fear of adding an undue number of planning cycles, but it's still better than what we have now. For ease of review, 0001 adds the new planner logic, while 0002 removes the now-dead executor support. There's one bit of dead code that I left in place for the moment, which is ruleutils.c's support for printing AlternativeSubPlans. I'm not sure if that's worth keeping or not --- it's dead code for normal use, but if someone tried to use ruleutils.c to print partially-planned expression trees, maybe there'd be a use for it? (It's also arguable that readfuncs.c's support is now dead code, but I have little interest in stripping that out.) regards, tom lane diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e2f177515d..f0386480ab 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2254,6 +2254,7 @@ _outPlannerInfo(StringInfo str, const PlannerInfo *node) WRITE_BOOL_FIELD(hasLateralRTEs); WRITE_BOOL_FIELD(hasHavingQual); WRITE_BOOL_FIELD(hasPseudoConstantQuals); + WRITE_BOOL_FIELD(hasAlternativeSubPlans); WRITE_BOOL_FIELD(hasRecursion); WRITE_INT_FIELD(wt_param_id); WRITE_BITMAPSET_FIELD(curOuterRels); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b40a112c25..27006c59e0 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -629,6 +629,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse, root->minmax_aggs = NIL; root->qual_security_level = 0; root->inhTargetKind = INHKIND_NONE; + root->hasPseudoConstantQuals = false; + root->hasAlternativeSubPlans = false; root->hasRecursion = hasRecursion; if (hasRecursion) root->wt_param_id = assign_special_exec_param(root); @@ -759,9 +761,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, */ root->hasHavingQual = (parse->havingQual != NULL); - /* Clear this flag; might get set in distribute_qual_to_rels */ - root->hasPseudoConstantQuals = false; - /* * Do expression preprocessing on targetlist and quals, as well as other * random expressions in the querytree. Note that we do not need to diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index baefe0e946..b5a29ecfc4 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -49,6 +49,7 @@ typedef struct { PlannerInfo *root; int rtoffset; + double num_exec; } fix_scan_expr_context; typedef struct @@ -58,6 +59,7 @@ typedef struct indexed_tlist *inner_itlist; Index acceptable_rel; int rtoffset; + double num_exec; } fix_join_expr_context; typedef struct @@ -66,8 +68,28 @@ typedef struct indexed_tlist *subplan_itlist; Index newvarno; int rtoffset; + double num_exec; } fix_upper_expr_context; +/* + * Selecting the best alternative in an AlternativeSubPlan expression requires + * estimating how many times that expression will be evaluated. For an + * expression in a plan node's targetlist, the plan's estimated number of + * output rows is clearly what to use, but for an expression in a qual it's + * far less clear. Since AlternativeSubPlans aren't heavily used, we don't + * want to expend a lot of cycles making such estimates. What we use is twice + * the number of output rows. That's not entirely unfounded: we know that + * clause_selectivity() would fall back to a default selectivity estimate + * of 0.5 for any SubPlan, so if the qual containing the SubPlan is the last + * to be applied (which it likely would be, thanks to order_qual_clauses()), + * this matches what we could have estimated in a far more laborious fashion. + * Obviously there are many other scenarios, but it's probably not worth the + * trouble to try to improve on this estimate, especially not when we don't + * have a better estimate for the selectivity of the SubPlan qual itself. + */ +#define NUM_EXEC_TLIST(parentplan) ((parentplan)->plan_rows) +#define NUM_EXEC_QUAL(parentplan) ((parentplan)->plan_rows * 2.0) + /* * Check if a Const node is a regclass value. We accept plain OID too, * since a regclass Const will get folded to that type if it's an argument @@ -79,8 +101,8 @@ typedef struct (((con)->consttype == REGCLASSOID || (con)->consttype == OIDOID) && \ !(con)->constisnull) -#define fix_scan_list(root, lst, rtoffset) \ - ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset)) +#define fix_scan_list(root, lst, rtoffset, num_exec) \ + ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset, num_exec)) static void add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing); static void flatten_unplanned_rtes(PlannerGlobal *glob, RangeTblEntry *rte); @@ -109,7 +131,8 @@ static Plan *set_mergeappend_references(PlannerInfo *root, int rtoffset); static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset); static Relids offset_relid_set(Relids relids, int rtoffset); -static Node *fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset); +static Node *fix_scan_expr(PlannerInfo *root, Node *node, + int rtoffset, double num_exec); static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context); static bool fix_scan_expr_walker(Node *node, fix_scan_expr_context *context); static void set_join_references(PlannerInfo *root, Join *join, int rtoffset); @@ -133,14 +156,15 @@ static List *fix_join_expr(PlannerInfo *root, List *clauses, indexed_tlist *outer_itlist, indexed_tlist *inner_itlist, - Index acceptable_rel, int rtoffset); + Index acceptable_rel, + int rtoffset, double num_exec); static Node *fix_join_expr_mutator(Node *node, fix_join_expr_context *context); static Node *fix_upper_expr(PlannerInfo *root, Node *node, indexed_tlist *subplan_itlist, Index newvarno, - int rtoffset); + int rtoffset, double num_exec); static Node *fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context); static List *set_returning_clause_references(PlannerInfo *root, @@ -177,17 +201,20 @@ static List *set_returning_clause_references(PlannerInfo *root, * 5. PARAM_MULTIEXPR Params are replaced by regular PARAM_EXEC Params, * now that we have finished planning all MULTIEXPR subplans. * - * 6. We compute regproc OIDs for operators (ie, we look up the function + * 6. AlternativeSubPlan expressions are replaced by just one of their + * alternatives, using an estimate of how many times they'll be executed. + * + * 7. We compute regproc OIDs for operators (ie, we look up the function * that implements each op). * - * 7. We create lists of specific objects that the plan depends on. + * 8. We create lists of specific objects that the plan depends on. * This will be used by plancache.c to drive invalidation of cached plans. * Relation dependencies are represented by OIDs, and everything else by * PlanInvalItems (this distinction is motivated by the shared-inval APIs). * Currently, relations, user-defined functions, and domains are the only * types of objects that are explicitly tracked this way. * - * 8. We assign every plan node in the tree a unique ID. + * 9. We assign every plan node in the tree a unique ID. * * We also perform one final optimization step, which is to delete * SubqueryScan, Append, and MergeAppend plan nodes that aren't doing @@ -490,9 +517,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scanrelid += rtoffset; splan->plan.targetlist = - fix_scan_list(root, splan->plan.targetlist, rtoffset); + fix_scan_list(root, splan->plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->plan.qual = - fix_scan_list(root, splan->plan.qual, rtoffset); + fix_scan_list(root, splan->plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_SampleScan: @@ -501,11 +530,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tablesample = (TableSampleClause *) - fix_scan_expr(root, (Node *) splan->tablesample, rtoffset); + fix_scan_expr(root, (Node *) splan->tablesample, + rtoffset, 1); } break; case T_IndexScan: @@ -514,17 +546,23 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->indexqual = - fix_scan_list(root, splan->indexqual, rtoffset); + fix_scan_list(root, splan->indexqual, + rtoffset, 1); splan->indexqualorig = - fix_scan_list(root, splan->indexqualorig, rtoffset); + fix_scan_list(root, splan->indexqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); splan->indexorderby = - fix_scan_list(root, splan->indexorderby, rtoffset); + fix_scan_list(root, splan->indexorderby, + rtoffset, 1); splan->indexorderbyorig = - fix_scan_list(root, splan->indexorderbyorig, rtoffset); + fix_scan_list(root, splan->indexorderbyorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_IndexOnlyScan: @@ -543,9 +581,10 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->scan.plan.targetlist == NIL); Assert(splan->scan.plan.qual == NIL); splan->indexqual = - fix_scan_list(root, splan->indexqual, rtoffset); + fix_scan_list(root, splan->indexqual, rtoffset, 1); splan->indexqualorig = - fix_scan_list(root, splan->indexqualorig, rtoffset); + fix_scan_list(root, splan->indexqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_BitmapHeapScan: @@ -554,11 +593,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->bitmapqualorig = - fix_scan_list(root, splan->bitmapqualorig, rtoffset); + fix_scan_list(root, splan->bitmapqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_TidScan: @@ -567,11 +609,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tidquals = - fix_scan_list(root, splan->tidquals, rtoffset); + fix_scan_list(root, splan->tidquals, + rtoffset, 1); } break; case T_SubqueryScan: @@ -585,11 +630,13 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->functions = - fix_scan_list(root, splan->functions, rtoffset); + fix_scan_list(root, splan->functions, rtoffset, 1); } break; case T_TableFuncScan: @@ -598,11 +645,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tablefunc = (TableFunc *) - fix_scan_expr(root, (Node *) splan->tablefunc, rtoffset); + fix_scan_expr(root, (Node *) splan->tablefunc, + rtoffset, 1); } break; case T_ValuesScan: @@ -611,11 +661,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->values_lists = - fix_scan_list(root, splan->values_lists, rtoffset); + fix_scan_list(root, splan->values_lists, + rtoffset, 1); } break; case T_CteScan: @@ -624,9 +677,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_NamedTuplestoreScan: @@ -635,9 +690,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_WorkTableScan: @@ -646,9 +703,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_ForeignScan: @@ -732,9 +791,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->plan.qual == NIL); splan->limitOffset = - fix_scan_expr(root, splan->limitOffset, rtoffset); + fix_scan_expr(root, splan->limitOffset, rtoffset, 1); splan->limitCount = - fix_scan_expr(root, splan->limitCount, rtoffset); + fix_scan_expr(root, splan->limitCount, rtoffset, 1); } break; case T_Agg: @@ -775,9 +834,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) * variable refs, so fix_scan_expr works for them. */ wplan->startOffset = - fix_scan_expr(root, wplan->startOffset, rtoffset); + fix_scan_expr(root, wplan->startOffset, rtoffset, 1); wplan->endOffset = - fix_scan_expr(root, wplan->endOffset, rtoffset); + fix_scan_expr(root, wplan->endOffset, rtoffset, 1); } break; case T_Result: @@ -793,13 +852,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) else { splan->plan.targetlist = - fix_scan_list(root, splan->plan.targetlist, rtoffset); + fix_scan_list(root, splan->plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->plan.qual = - fix_scan_list(root, splan->plan.qual, rtoffset); + fix_scan_list(root, splan->plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } /* resconstantqual can't contain any subplan variable refs */ splan->resconstantqual = - fix_scan_expr(root, splan->resconstantqual, rtoffset); + fix_scan_expr(root, splan->resconstantqual, rtoffset, 1); } break; case T_ProjectSet: @@ -813,7 +874,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->plan.qual == NIL); splan->withCheckOptionLists = - fix_scan_list(root, splan->withCheckOptionLists, rtoffset); + fix_scan_list(root, splan->withCheckOptionLists, + rtoffset, 1); if (splan->returningLists) { @@ -874,18 +936,18 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) fix_join_expr(root, splan->onConflictSet, NULL, itlist, linitial_int(splan->resultRelations), - rtoffset); + rtoffset, NUM_EXEC_QUAL(plan)); splan->onConflictWhere = (Node *) fix_join_expr(root, (List *) splan->onConflictWhere, NULL, itlist, linitial_int(splan->resultRelations), - rtoffset); + rtoffset, NUM_EXEC_QUAL(plan)); pfree(itlist); splan->exclRelTlist = - fix_scan_list(root, splan->exclRelTlist, rtoffset); + fix_scan_list(root, splan->exclRelTlist, rtoffset, 1); } splan->nominalRelation += rtoffset; @@ -1026,19 +1088,24 @@ set_indexonlyscan_references(PlannerInfo *root, (Node *) plan->scan.plan.targetlist, index_itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) plan)); plan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) plan->scan.plan.qual, index_itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) plan)); /* indexqual is already transformed to reference index columns */ - plan->indexqual = fix_scan_list(root, plan->indexqual, rtoffset); + plan->indexqual = fix_scan_list(root, plan->indexqual, + rtoffset, 1); /* indexorderby is already transformed to reference index columns */ - plan->indexorderby = fix_scan_list(root, plan->indexorderby, rtoffset); + plan->indexorderby = fix_scan_list(root, plan->indexorderby, + rtoffset, 1); /* indextlist must NOT be transformed to reference index columns */ - plan->indextlist = fix_scan_list(root, plan->indextlist, rtoffset); + plan->indextlist = fix_scan_list(root, plan->indextlist, + rtoffset, NUM_EXEC_TLIST((Plan *) plan)); pfree(index_itlist); @@ -1084,9 +1151,11 @@ set_subqueryscan_references(PlannerInfo *root, */ plan->scan.scanrelid += rtoffset; plan->scan.plan.targetlist = - fix_scan_list(root, plan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, plan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) plan)); plan->scan.plan.qual = - fix_scan_list(root, plan->scan.plan.qual, rtoffset); + fix_scan_list(root, plan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) plan)); result = (Plan *) plan; } @@ -1202,29 +1271,34 @@ set_foreignscan_references(PlannerInfo *root, (Node *) fscan->scan.plan.targetlist, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) fscan)); fscan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) fscan->scan.plan.qual, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_exprs = (List *) fix_upper_expr(root, (Node *) fscan->fdw_exprs, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_recheck_quals = (List *) fix_upper_expr(root, (Node *) fscan->fdw_recheck_quals, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); pfree(itlist); /* fdw_scan_tlist itself just needs fix_scan_list() adjustments */ fscan->fdw_scan_tlist = - fix_scan_list(root, fscan->fdw_scan_tlist, rtoffset); + fix_scan_list(root, fscan->fdw_scan_tlist, + rtoffset, NUM_EXEC_TLIST((Plan *) fscan)); } else { @@ -1233,13 +1307,17 @@ set_foreignscan_references(PlannerInfo *root, * way */ fscan->scan.plan.targetlist = - fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, fscan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) fscan)); fscan->scan.plan.qual = - fix_scan_list(root, fscan->scan.plan.qual, rtoffset); + fix_scan_list(root, fscan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_exprs = - fix_scan_list(root, fscan->fdw_exprs, rtoffset); + fix_scan_list(root, fscan->fdw_exprs, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_recheck_quals = - fix_scan_list(root, fscan->fdw_recheck_quals, rtoffset); + fix_scan_list(root, fscan->fdw_recheck_quals, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); } fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset); @@ -1270,33 +1348,40 @@ set_customscan_references(PlannerInfo *root, (Node *) cscan->scan.plan.targetlist, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) cscan)); cscan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) cscan->scan.plan.qual, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) cscan)); cscan->custom_exprs = (List *) fix_upper_expr(root, (Node *) cscan->custom_exprs, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) cscan)); pfree(itlist); /* custom_scan_tlist itself just needs fix_scan_list() adjustments */ cscan->custom_scan_tlist = - fix_scan_list(root, cscan->custom_scan_tlist, rtoffset); + fix_scan_list(root, cscan->custom_scan_tlist, + rtoffset, NUM_EXEC_TLIST((Plan *) cscan)); } else { /* Adjust tlist, qual, custom_exprs in the standard way */ cscan->scan.plan.targetlist = - fix_scan_list(root, cscan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, cscan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) cscan)); cscan->scan.plan.qual = - fix_scan_list(root, cscan->scan.plan.qual, rtoffset); + fix_scan_list(root, cscan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) cscan)); cscan->custom_exprs = - fix_scan_list(root, cscan->custom_exprs, rtoffset); + fix_scan_list(root, cscan->custom_exprs, + rtoffset, NUM_EXEC_QUAL((Plan *) cscan)); } /* Adjust child plan-nodes recursively, if needed */ @@ -1458,7 +1543,8 @@ set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) hplan->hashkeys, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL(plan)); /* Hash doesn't project */ set_dummy_tlist_references(plan, rtoffset); @@ -1623,6 +1709,40 @@ fix_param_node(PlannerInfo *root, Param *p) return (Node *) copyObject(p); } +/* + * fix_alternative_subplan + * Do set_plan_references processing on an AlternativeSubPlan + * + * Choose one of the alternative implementations and return just that one, + * discarding the rest of the AlternativeSubPlan structure. + * Note: caller must still recurse into the result! + * + * We don't make any attempt to fix up cost estimates in the parent + * plan node or higher-level nodes. + */ +static Node * +fix_alternative_subplan(AlternativeSubPlan *asplan, double num_exec) +{ + SubPlan *subplan1; + SubPlan *subplan2; + Cost cost1; + Cost cost2; + + /* Currently there will always be exactly 2 alternatives */ + Assert(list_length(asplan->subplans) == 2); + subplan1 = (SubPlan *) linitial(asplan->subplans); + subplan2 = (SubPlan *) lsecond(asplan->subplans); + + /* Choose the one that's cheaper assuming num_exec executions */ + cost1 = subplan1->startup_cost + num_exec * subplan1->per_call_cost; + cost2 = subplan2->startup_cost + num_exec * subplan2->per_call_cost; + + if (cost1 < cost2) + return (Node *) subplan1; + else + return (Node *) subplan2; +} + /* * fix_scan_expr * Do set_plan_references processing on a scan-level expression @@ -1630,21 +1750,24 @@ fix_param_node(PlannerInfo *root, Param *p) * This consists of incrementing all Vars' varnos by rtoffset, * replacing PARAM_MULTIEXPR Params, expanding PlaceHolderVars, * replacing Aggref nodes that should be replaced by initplan output Params, + * choosing the best implementation for AlternativeSubPlans, * looking up operator opcode info for OpExpr and related nodes, * and adding OIDs from regclass Const nodes into root->glob->relationOids. */ static Node * -fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset) +fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset, double num_exec) { fix_scan_expr_context context; context.root = root; context.rtoffset = rtoffset; + context.num_exec = num_exec; if (rtoffset != 0 || root->multiexpr_params != NIL || root->glob->lastPHId != 0 || - root->minmax_aggs != NIL) + root->minmax_aggs != NIL || + root->hasAlternativeSubPlans) { return fix_scan_expr_mutator(node, &context); } @@ -1655,7 +1778,8 @@ fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset) * are no MULTIEXPR subqueries then we don't need to replace * PARAM_MULTIEXPR Params, and if there are no placeholders anywhere * we won't need to remove them, and if there are no minmax Aggrefs we - * won't need to replace them. Then it's OK to just scribble on the + * won't need to replace them, and if there are no AlternativeSubPlans + * we won't need to remove them. Then it's OK to just scribble on the * input node tree instead of copying (since the only change, filling * in any unset opfuncid fields, is harmless). This saves just enough * cycles to be noticeable on trivial queries. @@ -1729,6 +1853,10 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context) return fix_scan_expr_mutator((Node *) phv->phexpr, context); } + if (IsA(node, AlternativeSubPlan)) + return fix_scan_expr_mutator(fix_alternative_subplan((AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_scan_expr_mutator, (void *) context); @@ -1740,6 +1868,7 @@ fix_scan_expr_walker(Node *node, fix_scan_expr_context *context) if (node == NULL) return false; Assert(!IsA(node, PlaceHolderVar)); + Assert(!IsA(node, AlternativeSubPlan)); fix_expr_common(context->root, node); return expression_tree_walker(node, fix_scan_expr_walker, (void *) context); @@ -1776,7 +1905,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); /* Now do join-type-specific stuff */ if (IsA(join, NestLoop)) @@ -1792,7 +1922,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) (Node *) nlp->paramval, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(outer_plan)); /* Check we replaced any PlaceHolderVar with simple Var */ if (!(IsA(nlp->paramval, Var) && nlp->paramval->varno == OUTER_VAR)) @@ -1808,7 +1939,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); } else if (IsA(join, HashJoin)) { @@ -1819,7 +1951,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); /* * HashJoin's hashkeys are used to look for matching tuples from its @@ -1829,7 +1962,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) (Node *) hj->hashkeys, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); } /* @@ -1867,13 +2001,15 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) join)); join->plan.qual = fix_join_expr(root, join->plan.qual, outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); pfree(outer_itlist); pfree(inner_itlist); @@ -1926,14 +2062,16 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) tle->expr, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(plan)); } else newexpr = fix_upper_expr(root, (Node *) tle->expr, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(plan)); tle = flatCopyTargetEntry(tle); tle->expr = (Expr *) newexpr; output_targetlist = lappend(output_targetlist, tle); @@ -1945,7 +2083,8 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) plan->qual, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL(plan)); pfree(subplan_itlist); } @@ -2389,6 +2528,7 @@ search_indexed_tlist_for_sortgroupref(Expr *node, * 'acceptable_rel' is either zero or the rangetable index of a relation * whose Vars may appear in the clause without provoking an error * 'rtoffset': how much to increment varnos by + * 'num_exec': estimated number of executions of expression * * Returns the new expression tree. The original clause structure is * not modified. @@ -2399,7 +2539,8 @@ fix_join_expr(PlannerInfo *root, indexed_tlist *outer_itlist, indexed_tlist *inner_itlist, Index acceptable_rel, - int rtoffset) + int rtoffset, + double num_exec) { fix_join_expr_context context; @@ -2408,6 +2549,7 @@ fix_join_expr(PlannerInfo *root, context.inner_itlist = inner_itlist; context.acceptable_rel = acceptable_rel; context.rtoffset = rtoffset; + context.num_exec = num_exec; return (List *) fix_join_expr_mutator((Node *) clauses, &context); } @@ -2502,6 +2644,10 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) /* Special cases (apply only AFTER failing to match to lower tlist) */ if (IsA(node, Param)) return fix_param_node(context->root, (Param *) node); + if (IsA(node, AlternativeSubPlan)) + return fix_join_expr_mutator(fix_alternative_subplan((AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_join_expr_mutator, @@ -2533,6 +2679,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) * 'subplan_itlist': indexed target list for subplan (or index) * 'newvarno': varno to use for Vars referencing tlist elements * 'rtoffset': how much to increment varnos by + * 'num_exec': estimated number of executions of expression * * The resulting tree is a copy of the original in which all Var nodes have * varno = newvarno, varattno = resno of corresponding targetlist element. @@ -2543,7 +2690,8 @@ fix_upper_expr(PlannerInfo *root, Node *node, indexed_tlist *subplan_itlist, Index newvarno, - int rtoffset) + int rtoffset, + double num_exec) { fix_upper_expr_context context; @@ -2551,6 +2699,7 @@ fix_upper_expr(PlannerInfo *root, context.subplan_itlist = subplan_itlist; context.newvarno = newvarno; context.rtoffset = rtoffset; + context.num_exec = num_exec; return fix_upper_expr_mutator(node, &context); } @@ -2623,6 +2772,10 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context) } /* If no match, just fall through to process it normally */ } + if (IsA(node, AlternativeSubPlan)) + return fix_upper_expr_mutator(fix_alternative_subplan((AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_upper_expr_mutator, @@ -2687,7 +2840,8 @@ set_returning_clause_references(PlannerInfo *root, itlist, NULL, resultRelation, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(topplan)); pfree(itlist); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 6eb794669f..79cc2a386d 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -247,7 +247,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, * likely to be better (it depends on the expected number of executions of * the EXISTS qual, and we are much too early in planning the outer query * to be able to guess that). So we generate both plans, if possible, and - * leave it to the executor to decide which to use. + * leave it to setrefs.c to decide which to use. */ if (simple_exists && IsA(result, SubPlan)) { @@ -298,10 +298,11 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, Assert(hashplan->parParam == NIL); Assert(hashplan->useHashTable); - /* Leave it to the executor to decide which plan to use */ + /* Leave it to setrefs.c to decide which plan to use */ asplan = makeNode(AlternativeSubPlan); asplan->subplans = list_make2(result, hashplan); result = (Node *) asplan; + root->hasAlternativeSubPlans = true; } } } diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 485d1b06c9..dbe86e7af6 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -347,6 +347,7 @@ struct PlannerInfo bool hasHavingQual; /* true if havingQual was non-null */ bool hasPseudoConstantQuals; /* true if any RestrictInfo has * pseudoconstant = true */ + bool hasAlternativeSubPlans; /* true if we've made any of those */ bool hasRecursion; /* true if planning a recursive WITH item */ /* These fields are used only when hasRecursion is true: */ diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index d73be2ad46..fd65ee8f9c 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -736,6 +736,9 @@ typedef struct SubPlan /* * AlternativeSubPlan - expression node for a choice among SubPlans * + * This is used only transiently during planning: by the time the plan + * reaches the executor, all AlternativeSubPlan nodes have been removed. + * * The subplans are given as a List so that the node definition need not * change if there's ever more than two alternatives. For the moment, * though, there are always exactly two; and the first one is the fast-start diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 1338b2b23e..ff157ceb1c 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -50,14 +50,12 @@ explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on con Insert on insertconflicttest Conflict Resolution: UPDATE Conflict Arbiter Indexes: op_index_key, collation_index_key, both_index_key - Conflict Filter: (alternatives: SubPlan 1 or hashed SubPlan 2) + Conflict Filter: (SubPlan 1) -> Result SubPlan 1 -> Index Only Scan using both_index_expr_key on insertconflicttest ii Index Cond: (key = excluded.key) - SubPlan 2 - -> Seq Scan on insertconflicttest ii_1 -(10 rows) +(8 rows) -- Neither collation nor operator class specifications are required -- -- supplying them merely *limits* matches to indexes with matching opclasses diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index b81923f2e7..9d56cdacf3 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -874,6 +874,53 @@ select * from int8_tbl where q1 in (select c1 from inner_text); (2 rows) rollback; -- to get rid of the bogus operator +-- +-- Test resolution of hashed vs non-hashed implementation of EXISTS subplan +-- +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + QUERY PLAN +-------------------------------------------------------------- + Aggregate + -> Seq Scan on tenk1 t + Filter: ((hashed SubPlan 2) OR (ten < 0)) + SubPlan 2 + -> Index Only Scan using tenk1_unique1 on tenk1 k +(5 rows) + +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + count +------- + 10000 +(1 row) + +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + QUERY PLAN +-------------------------------------------------------------- + Aggregate + -> Bitmap Heap Scan on tenk1 t + Recheck Cond: (thousand = 1) + Filter: ((SubPlan 1) OR (ten < 0)) + -> Bitmap Index Scan on tenk1_thous_tenthous + Index Cond: (thousand = 1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on tenk1 k + Index Cond: (unique1 = t.unique2) +(9 rows) + +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + count +------- + 10 +(1 row) + -- -- Test case for planner bug with nested EXISTS handling -- diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 5de53f2782..caed1c19ec 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -1869,9 +1869,7 @@ EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (5); SubPlan 1 -> Index Only Scan using ref_tbl_pkey on ref_tbl r Index Cond: (a = b.a) - SubPlan 2 - -> Seq Scan on ref_tbl r_1 -(7 rows) +(5 rows) EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; QUERY PLAN @@ -1885,9 +1883,7 @@ EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; SubPlan 1 -> Index Only Scan using ref_tbl_pkey on ref_tbl r_1 Index Cond: (a = b.a) - SubPlan 2 - -> Seq Scan on ref_tbl r_2 -(11 rows) +(9 rows) DROP TABLE base_tbl, ref_tbl CASCADE; NOTICE: drop cascades to view rw_view1 @@ -2301,8 +2297,8 @@ SELECT * FROM v1 WHERE a=8; EXPLAIN (VERBOSE, COSTS OFF) UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; - QUERY PLAN ---------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------------------------------- Update on public.t1 Update on public.t1 Update on public.t11 t1_1 @@ -2311,32 +2307,26 @@ UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; -> Index Scan using t1_a_idx on public.t1 Output: 100, t1.b, t1.c, t1.ctid Index Cond: ((t1.a > 5) AND (t1.a < 7)) - Filter: ((t1.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1.a) AND leakproof(t1.a)) + Filter: ((t1.a <> 6) AND (SubPlan 1) AND snoop(t1.a) AND leakproof(t1.a)) SubPlan 1 -> Append -> Seq Scan on public.t12 t12_1 Filter: (t12_1.a = t1.a) -> Seq Scan on public.t111 t12_2 Filter: (t12_2.a = t1.a) - SubPlan 2 - -> Append - -> Seq Scan on public.t12 t12_4 - Output: t12_4.a - -> Seq Scan on public.t111 t12_5 - Output: t12_5.a -> Index Scan using t11_a_idx on public.t11 t1_1 Output: 100, t1_1.b, t1_1.c, t1_1.d, t1_1.ctid Index Cond: ((t1_1.a > 5) AND (t1_1.a < 7)) - Filter: ((t1_1.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_1.a) AND leakproof(t1_1.a)) + Filter: ((t1_1.a <> 6) AND (SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a)) -> Index Scan using t12_a_idx on public.t12 t1_2 Output: 100, t1_2.b, t1_2.c, t1_2.e, t1_2.ctid Index Cond: ((t1_2.a > 5) AND (t1_2.a < 7)) - Filter: ((t1_2.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_2.a) AND leakproof(t1_2.a)) + Filter: ((t1_2.a <> 6) AND (SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a)) -> Index Scan using t111_a_idx on public.t111 t1_3 Output: 100, t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid Index Cond: ((t1_3.a > 5) AND (t1_3.a < 7)) - Filter: ((t1_3.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_3.a) AND leakproof(t1_3.a)) -(33 rows) + Filter: ((t1_3.a <> 6) AND (SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a)) +(27 rows) UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; SELECT * FROM v1 WHERE a=100; -- Nothing should have been changed to 100 @@ -2351,8 +2341,8 @@ SELECT * FROM t1 WHERE a=100; -- Nothing should have been changed to 100 EXPLAIN (VERBOSE, COSTS OFF) UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; - QUERY PLAN ---------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------- Update on public.t1 Update on public.t1 Update on public.t11 t1_1 @@ -2361,32 +2351,26 @@ UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; -> Index Scan using t1_a_idx on public.t1 Output: (t1.a + 1), t1.b, t1.c, t1.ctid Index Cond: ((t1.a > 5) AND (t1.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1.a) AND leakproof(t1.a)) + Filter: ((SubPlan 1) AND snoop(t1.a) AND leakproof(t1.a)) SubPlan 1 -> Append -> Seq Scan on public.t12 t12_1 Filter: (t12_1.a = t1.a) -> Seq Scan on public.t111 t12_2 Filter: (t12_2.a = t1.a) - SubPlan 2 - -> Append - -> Seq Scan on public.t12 t12_4 - Output: t12_4.a - -> Seq Scan on public.t111 t12_5 - Output: t12_5.a -> Index Scan using t11_a_idx on public.t11 t1_1 Output: (t1_1.a + 1), t1_1.b, t1_1.c, t1_1.d, t1_1.ctid Index Cond: ((t1_1.a > 5) AND (t1_1.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_1.a) AND leakproof(t1_1.a)) + Filter: ((SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a)) -> Index Scan using t12_a_idx on public.t12 t1_2 Output: (t1_2.a + 1), t1_2.b, t1_2.c, t1_2.e, t1_2.ctid Index Cond: ((t1_2.a > 5) AND (t1_2.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_2.a) AND leakproof(t1_2.a)) + Filter: ((SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a)) -> Index Scan using t111_a_idx on public.t111 t1_3 Output: (t1_3.a + 1), t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid Index Cond: ((t1_3.a > 5) AND (t1_3.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_3.a) AND leakproof(t1_3.a)) -(33 rows) + Filter: ((SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a)) +(27 rows) UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; NOTICE: snooped value: 8 diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index cce8ebdb3d..a25cb6fc5c 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -509,6 +509,23 @@ select * from int8_tbl where q1 in (select c1 from inner_text); rollback; -- to get rid of the bogus operator +-- +-- Test resolution of hashed vs non-hashed implementation of EXISTS subplan +-- +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + -- -- Test case for planner bug with nested EXISTS handling -- diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 236413f62a..868f8b0858 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1104,23 +1104,6 @@ ExecInitExprRec(Expr *node, ExprState *state, break; } - case T_AlternativeSubPlan: - { - AlternativeSubPlan *asplan = (AlternativeSubPlan *) node; - AlternativeSubPlanState *asstate; - - if (!state->parent) - elog(ERROR, "AlternativeSubPlan found with no parent plan"); - - asstate = ExecInitAlternativeSubPlan(asplan, state->parent); - - scratch.opcode = EEOP_ALTERNATIVE_SUBPLAN; - scratch.d.alternative_subplan.asstate = asstate; - - ExprEvalPushStep(state, &scratch); - break; - } - case T_FieldSelect: { FieldSelect *fselect = (FieldSelect *) node; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index b812bbacee..26c2b49632 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -431,7 +431,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_GROUPING_FUNC, &&CASE_EEOP_WINDOW_FUNC, &&CASE_EEOP_SUBPLAN, - &&CASE_EEOP_ALTERNATIVE_SUBPLAN, &&CASE_EEOP_AGG_STRICT_DESERIALIZE, &&CASE_EEOP_AGG_DESERIALIZE, &&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS, @@ -1536,14 +1535,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } - EEO_CASE(EEOP_ALTERNATIVE_SUBPLAN) - { - /* too complex for an inline implementation */ - ExecEvalAlternativeSubPlan(state, op, econtext); - - EEO_NEXT(); - } - /* evaluate a strict aggregate deserialization function */ EEO_CASE(EEOP_AGG_STRICT_DESERIALIZE) { @@ -3868,20 +3859,6 @@ ExecEvalSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext) *op->resvalue = ExecSubPlan(sstate, econtext, op->resnull); } -/* - * Hand off evaluation of an alternative subplan to nodeSubplan.c - */ -void -ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext) -{ - AlternativeSubPlanState *asstate = op->d.alternative_subplan.asstate; - - /* could potentially be nested, so make sure there's enough stack */ - check_stack_depth(); - - *op->resvalue = ExecAlternativeSubPlan(asstate, econtext, op->resnull); -} - /* * Evaluate a wholerow Var expression. * diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 9a7962518e..9a706df5f0 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -1303,83 +1303,3 @@ ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent) parent->chgParam = bms_add_member(parent->chgParam, paramid); } } - - -/* - * ExecInitAlternativeSubPlan - * - * Initialize for execution of one of a set of alternative subplans. - */ -AlternativeSubPlanState * -ExecInitAlternativeSubPlan(AlternativeSubPlan *asplan, PlanState *parent) -{ - AlternativeSubPlanState *asstate = makeNode(AlternativeSubPlanState); - double num_calls; - SubPlan *subplan1; - SubPlan *subplan2; - Cost cost1; - Cost cost2; - ListCell *lc; - - asstate->subplan = asplan; - - /* - * Initialize subplans. (Can we get away with only initializing the one - * we're going to use?) - */ - foreach(lc, asplan->subplans) - { - SubPlan *sp = lfirst_node(SubPlan, lc); - SubPlanState *sps = ExecInitSubPlan(sp, parent); - - asstate->subplans = lappend(asstate->subplans, sps); - parent->subPlan = lappend(parent->subPlan, sps); - } - - /* - * Select the one to be used. For this, we need an estimate of the number - * of executions of the subplan. We use the number of output rows - * expected from the parent plan node. This is a good estimate if we are - * in the parent's targetlist, and an underestimate (but probably not by - * more than a factor of 2) if we are in the qual. - */ - num_calls = parent->plan->plan_rows; - - /* - * The planner saved enough info so that we don't have to work very hard - * to estimate the total cost, given the number-of-calls estimate. - */ - Assert(list_length(asplan->subplans) == 2); - subplan1 = (SubPlan *) linitial(asplan->subplans); - subplan2 = (SubPlan *) lsecond(asplan->subplans); - - cost1 = subplan1->startup_cost + num_calls * subplan1->per_call_cost; - cost2 = subplan2->startup_cost + num_calls * subplan2->per_call_cost; - - if (cost1 < cost2) - asstate->active = 0; - else - asstate->active = 1; - - return asstate; -} - -/* - * ExecAlternativeSubPlan - * - * Execute one of a set of alternative subplans. - * - * Note: in future we might consider changing to different subplans on the - * fly, in case the original rowcount estimate turns out to be way off. - */ -Datum -ExecAlternativeSubPlan(AlternativeSubPlanState *node, - ExprContext *econtext, - bool *isNull) -{ - /* Just pass control to the active subplan */ - SubPlanState *activesp = list_nth_node(SubPlanState, - node->subplans, node->active); - - return ExecSubPlan(activesp, econtext, isNull); -} diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index cca5c117a0..eb1dea658c 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1918,12 +1918,6 @@ llvm_compile_expr(ExprState *state) LLVMBuildBr(b, opblocks[opno + 1]); break; - case EEOP_ALTERNATIVE_SUBPLAN: - build_EvalXFunc(b, mod, "ExecEvalAlternativeSubPlan", - v_state, op, v_econtext); - LLVMBuildBr(b, opblocks[opno + 1]); - break; - case EEOP_AGG_STRICT_DESERIALIZE: case EEOP_AGG_DESERIALIZE: { diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index 0a93d5f665..1ed3cafa2f 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -102,7 +102,6 @@ void *referenced_functions[] = ExecAggTransReparent, ExecEvalAggOrderedTransDatum, ExecEvalAggOrderedTransTuple, - ExecEvalAlternativeSubPlan, ExecEvalArrayCoerce, ExecEvalArrayExpr, ExecEvalConstraintCheck, diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 60dd80c23c..0e3084325e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8192,7 +8192,12 @@ get_rule_expr(Node *node, deparse_context *context, AlternativeSubPlan *asplan = (AlternativeSubPlan *) node; ListCell *lc; - /* As above, this can only happen during EXPLAIN */ + /* + * This case cannot be reached in normal usage, since no + * AlternativeSubPlan can appear either in parsetrees or + * finished plan trees. We keep it just in case somebody + * wants to use this code to print planner data structures. + */ appendStringInfoString(buf, "(alternatives: "); foreach(lc, asplan->subplans) { diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index dbe8649a57..b792de1bc9 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -218,7 +218,6 @@ typedef enum ExprEvalOp EEOP_GROUPING_FUNC, EEOP_WINDOW_FUNC, EEOP_SUBPLAN, - EEOP_ALTERNATIVE_SUBPLAN, /* aggregation related nodes */ EEOP_AGG_STRICT_DESERIALIZE, @@ -589,13 +588,6 @@ typedef struct ExprEvalStep SubPlanState *sstate; } subplan; - /* for EEOP_ALTERNATIVE_SUBPLAN */ - struct - { - /* out-of-line state, created by nodeSubplan.c */ - AlternativeSubPlanState *asstate; - } alternative_subplan; - /* for EEOP_AGG_*DESERIALIZE */ struct { @@ -734,8 +726,6 @@ extern void ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op); extern void ExecEvalGroupingFunc(ExprState *state, ExprEvalStep *op); extern void ExecEvalSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext); -extern void ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op, - ExprContext *econtext); extern void ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalSysVar(ExprState *state, ExprEvalStep *op, diff --git a/src/include/executor/nodeSubplan.h b/src/include/executor/nodeSubplan.h index 83e90b3d07..b629af1f5f 100644 --- a/src/include/executor/nodeSubplan.h +++ b/src/include/executor/nodeSubplan.h @@ -18,12 +18,8 @@ extern SubPlanState *ExecInitSubPlan(SubPlan *subplan, PlanState *parent); -extern AlternativeSubPlanState *ExecInitAlternativeSubPlan(AlternativeSubPlan *asplan, PlanState *parent); - extern Datum ExecSubPlan(SubPlanState *node, ExprContext *econtext, bool *isNull); -extern Datum ExecAlternativeSubPlan(AlternativeSubPlanState *node, ExprContext *econtext, bool *isNull); - extern void ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent); extern void ExecSetParamPlan(SubPlanState *node, ExprContext *econtext); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 0b42dd6f94..7e02afa6ba 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -880,18 +880,6 @@ typedef struct SubPlanState ExprState *cur_eq_comp; /* equality comparator for LHS vs. table */ } SubPlanState; -/* ---------------- - * AlternativeSubPlanState node - * ---------------- - */ -typedef struct AlternativeSubPlanState -{ - NodeTag type; - AlternativeSubPlan *subplan; /* expression plan node */ - List *subplans; /* SubPlanStates of alternative subplans */ - int active; /* list index of the one we're using */ -} AlternativeSubPlanState; - /* * DomainConstraintState - one item to check during CoerceToDomain * diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index 381d84b4e4..7ddd8c011b 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -213,7 +213,6 @@ typedef enum NodeTag T_WindowFuncExprState, T_SetExprState, T_SubPlanState, - T_AlternativeSubPlanState, T_DomainConstraintState, /*
On Sun, Aug 30, 2020 at 7:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
> Back in bd3daddaf232d95b0c9ba6f99b0170a0147dd8af, which introduced
> AlternativeSubPlans, I wrote:
> There is a lot more that could be done based on this infrastructure: in
> particular it's interesting to consider switching to the hash plan if we start
> out using the non-hashed plan but find a lot more upper rows going by than we
> expected. I have therefore left some minor inefficiencies in place, such as
> initializing both subplans even though we will currently only use one.
>
> That commit will be twelve years old come August, and nobody has either
> built anything else atop it or shown any interest in making the plan choice
> switchable mid-run. So it seems like kind of a failed experiment.
>
> Therefore, I'm considering the idea of ripping out all executor support
> for AlternativeSubPlan and instead having the planner replace an
> AlternativeSubPlan with the desired specific SubPlan somewhere late in
> planning, possibly setrefs.c.
Here's a proposed patchset for that. This runs with the idea I'd had
that setrefs.c could be smarter than the executor about which plan node
subexpressions will be executed how many times. I did not take it very
far, for fear of adding an undue number of planning cycles, but it's still
better than what we have now.
For ease of review, 0001 adds the new planner logic, while 0002 removes
the now-dead executor support.
There's one bit of dead code that I left in place for the moment, which is
ruleutils.c's support for printing AlternativeSubPlans. I'm not sure if
that's worth keeping or not --- it's dead code for normal use, but if
someone tried to use ruleutils.c to print partially-planned expression
trees, maybe there'd be a use for it?
(It's also arguable that readfuncs.c's support is now dead code, but
I have little interest in stripping that out.)
regards, tom lane
should be executed when a join is involved. I care about this because this
has an impact on when we can get the num_exec for a subplan.
The subplan in a target list, it is executed after the join in my case. The subplan
can be execute after the scan of T1(see below example) and it can also be executed
after the join. Which one is better depends on which methods make the num_exec
smaller. Is it something we already considered? I drill-down to
populate_joinrel_with_paths and not find this logic.
# explain (costs off) select (select a from t2 where t2.b = t1.b) from t1, t3;
QUERY PLAN
------------------------------
Nested Loop
-> Seq Scan on t1
-> Materialize
-> Seq Scan on t3
SubPlan 1
-> Seq Scan on t2
Filter: (b = t1.b)
(7 rows)
QUERY PLAN
------------------------------
Nested Loop
-> Seq Scan on t1
-> Materialize
-> Seq Scan on t3
SubPlan 1
-> Seq Scan on t2
Filter: (b = t1.b)
(7 rows)
When the subplan is in a Qual, it is supposed to be executed as soon as possible,
The current implementation matches the below cases. So can we say we
knows the num_execs of SubPlan just after we plan the dependent rels?
(In Q1 below the dependent rel is t1 vs t3, in Q2 it is t1 only) If we can choose
a subplan and recost the related path during(not after) creating the best path, will
we get better results for some cases (due to the current cost method for AlternativeSubPlan[1])?
-- the subplan depends on the result of t1 join t3
# explain (costs off) select t1.* from t1, t3 where
t1.a > (select max(a) from t2 where t2.b = t1.b and t2.c = t3.c);
QUERY PLAN
-----------------------------------------------------
Nested Loop
Join Filter: (t1.a > (SubPlan 1))
-> Seq Scan on t1
-> Materialize
-> Seq Scan on t3
SubPlan 1
-> Aggregate
-> Seq Scan on t2
Filter: ((b = t1.b) AND (c = t3.c))
(9 rows)
-- the subplan only depends on t1.
# explain (costs off) select t1.* from t1, t3 where
QUERY PLAN
-----------------------------------------------------
Nested Loop
Join Filter: (t1.a > (SubPlan 1))
-> Seq Scan on t1
-> Materialize
-> Seq Scan on t3
SubPlan 1
-> Aggregate
-> Seq Scan on t2
Filter: ((b = t1.b) AND (c = t3.c))
(9 rows)
-- the subplan only depends on t1.
# explain (costs off) select t1.* from t1, t3 where
t1.a > (select max(a) from t2 where t2.b = t1.b);
QUERY PLAN
------------------------------------------------
Nested Loop
-> Seq Scan on t3
-> Materialize
-> Seq Scan on t1
Filter: (a > (SubPlan 1))
SubPlan 1
-> Aggregate
-> Seq Scan on t2
Filter: (b = t1.b)
(9 rows)
QUERY PLAN
------------------------------------------------
Nested Loop
-> Seq Scan on t3
-> Materialize
-> Seq Scan on t1
Filter: (a > (SubPlan 1))
SubPlan 1
-> Aggregate
-> Seq Scan on t2
Filter: (b = t1.b)
(9 rows)
At last, I want to use the commonly used table in src/test/regress/sql/create_table.sql
when providing an example, but I always have issues running the create_table.sql which
makes me uncomfortable to use that. Am I missing something?
CREATE TABLE fail_part PARTITION OF range_parted3 FOR VALUES FROM (1, minvalue) TO (1, maxvalue);
psql:src/test/regress/sql/create_table.sql:611: ERROR: partition "fail_part" would overlap partition "part10"
psql:src/test/regress/sql/create_table.sql:611: ERROR: partition "fail_part" would overlap partition "part10"
CREATE TABLE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 1);
psql:src/test/regress/sql/create_table.sql:622: ERROR: partition "fail_part" would overlap partition "h2part_4"
psql:src/test/regress/sql/create_table.sql:622: ERROR: partition "fail_part" would overlap partition "h2part_4"
Best Regards
Andy Fan
On Sun, 30 Aug 2020 at 11:26, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: > > Therefore, I'm considering the idea of ripping out all executor support > > for AlternativeSubPlan and instead having the planner replace an > > AlternativeSubPlan with the desired specific SubPlan somewhere late in > > planning, possibly setrefs.c. > > Here's a proposed patchset for that. Do you feel that the choice to create_plan() on the subplan before planning the outer query is still a good one? ISTM that that was required when the AlternativeSubplan decision was made during execution, since we, of course, need a plan to execute. If the decision is now being made in the planner then is it not better to delay the create_plan() until later in planning? From looking at the code it seems that Paths won't really do here as we're dealing with two separate PlannerInfos rather than two paths belonging to the same PlannerInfo, but maybe it's better to invent something else that's similar to a list of paths and just do create_plan() for the subquery once. David
David Rowley <dgrowleyml@gmail.com> writes: > Do you feel that the choice to create_plan() on the subplan before > planning the outer query is still a good one? ISTM that that was > required when the AlternativeSubplan decision was made during > execution, since we, of course, need a plan to execute. If the > decision is now being made in the planner then is it not better to > delay the create_plan() until later in planning? Hm. That's well outside the scope I had in mind for this patch. In principle, you're right that we could postpone final planning of the subquery till later; but I fear it'd require quite a lot of refactoring to make it work that way. There's a lot of rather subtle timing dependencies in the processing done by createplan.c and setrefs.c, so I think this might be a lot more painful than it seems at first glance. And we'd only gain anything in cases that use AlternativeSubPlan, which is a minority of subplans, so on the whole I rather doubt it's worth the trouble. One inefficiency I see that we could probably get rid of is where make_subplan() is doing /* Now we can check if it'll fit in hash_mem */ /* XXX can we check this at the Path stage? */ if (subplan_is_hashable(plan)) { The only inputs subplan_is_hashable needs are the predicted rowcount and output width, which surely we could get from the Path. So we could save doing create_plan() when we decide the subquery output is too big to hash. OTOH, that's probably a pretty small minority of use-cases, so it might not be worth troubling over. regards, tom lane
I wrote: > One inefficiency I see that we could probably get rid of is > where make_subplan() is doing > /* Now we can check if it'll fit in hash_mem */ > /* XXX can we check this at the Path stage? */ I went ahead and fixed that, and I also realized there's another small improvement to be made: we can remove the unused SubPlan from the subplans list of the finished PlannedStmt, by setting that list cell to NULL. (This is already specifically allowed by the comments for PlannedStmt.subplans.) Initially I supposed that this'd only save the costs of copying that subtree when we copy the whole plan. On looking closer, though, InitPlan actually runs ExecInitNode on every list entry, even unused ones, so this will make some difference in executor startup time. Hence, an updated 0001 patch. 0002 hasn't changed. regards, tom lane diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e2f177515d..f0386480ab 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2254,6 +2254,7 @@ _outPlannerInfo(StringInfo str, const PlannerInfo *node) WRITE_BOOL_FIELD(hasLateralRTEs); WRITE_BOOL_FIELD(hasHavingQual); WRITE_BOOL_FIELD(hasPseudoConstantQuals); + WRITE_BOOL_FIELD(hasAlternativeSubPlans); WRITE_BOOL_FIELD(hasRecursion); WRITE_INT_FIELD(wt_param_id); WRITE_BITMAPSET_FIELD(curOuterRels); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b40a112c25..27006c59e0 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -629,6 +629,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse, root->minmax_aggs = NIL; root->qual_security_level = 0; root->inhTargetKind = INHKIND_NONE; + root->hasPseudoConstantQuals = false; + root->hasAlternativeSubPlans = false; root->hasRecursion = hasRecursion; if (hasRecursion) root->wt_param_id = assign_special_exec_param(root); @@ -759,9 +761,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, */ root->hasHavingQual = (parse->havingQual != NULL); - /* Clear this flag; might get set in distribute_qual_to_rels */ - root->hasPseudoConstantQuals = false; - /* * Do expression preprocessing on targetlist and quals, as well as other * random expressions in the querytree. Note that we do not need to diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index baefe0e946..0355b52ed4 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -49,6 +49,7 @@ typedef struct { PlannerInfo *root; int rtoffset; + double num_exec; } fix_scan_expr_context; typedef struct @@ -58,6 +59,7 @@ typedef struct indexed_tlist *inner_itlist; Index acceptable_rel; int rtoffset; + double num_exec; } fix_join_expr_context; typedef struct @@ -66,8 +68,28 @@ typedef struct indexed_tlist *subplan_itlist; Index newvarno; int rtoffset; + double num_exec; } fix_upper_expr_context; +/* + * Selecting the best alternative in an AlternativeSubPlan expression requires + * estimating how many times that expression will be evaluated. For an + * expression in a plan node's targetlist, the plan's estimated number of + * output rows is clearly what to use, but for an expression in a qual it's + * far less clear. Since AlternativeSubPlans aren't heavily used, we don't + * want to expend a lot of cycles making such estimates. What we use is twice + * the number of output rows. That's not entirely unfounded: we know that + * clause_selectivity() would fall back to a default selectivity estimate + * of 0.5 for any SubPlan, so if the qual containing the SubPlan is the last + * to be applied (which it likely would be, thanks to order_qual_clauses()), + * this matches what we could have estimated in a far more laborious fashion. + * Obviously there are many other scenarios, but it's probably not worth the + * trouble to try to improve on this estimate, especially not when we don't + * have a better estimate for the selectivity of the SubPlan qual itself. + */ +#define NUM_EXEC_TLIST(parentplan) ((parentplan)->plan_rows) +#define NUM_EXEC_QUAL(parentplan) ((parentplan)->plan_rows * 2.0) + /* * Check if a Const node is a regclass value. We accept plain OID too, * since a regclass Const will get folded to that type if it's an argument @@ -79,8 +101,8 @@ typedef struct (((con)->consttype == REGCLASSOID || (con)->consttype == OIDOID) && \ !(con)->constisnull) -#define fix_scan_list(root, lst, rtoffset) \ - ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset)) +#define fix_scan_list(root, lst, rtoffset, num_exec) \ + ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset, num_exec)) static void add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing); static void flatten_unplanned_rtes(PlannerGlobal *glob, RangeTblEntry *rte); @@ -109,7 +131,8 @@ static Plan *set_mergeappend_references(PlannerInfo *root, int rtoffset); static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset); static Relids offset_relid_set(Relids relids, int rtoffset); -static Node *fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset); +static Node *fix_scan_expr(PlannerInfo *root, Node *node, + int rtoffset, double num_exec); static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context); static bool fix_scan_expr_walker(Node *node, fix_scan_expr_context *context); static void set_join_references(PlannerInfo *root, Join *join, int rtoffset); @@ -133,14 +156,15 @@ static List *fix_join_expr(PlannerInfo *root, List *clauses, indexed_tlist *outer_itlist, indexed_tlist *inner_itlist, - Index acceptable_rel, int rtoffset); + Index acceptable_rel, + int rtoffset, double num_exec); static Node *fix_join_expr_mutator(Node *node, fix_join_expr_context *context); static Node *fix_upper_expr(PlannerInfo *root, Node *node, indexed_tlist *subplan_itlist, Index newvarno, - int rtoffset); + int rtoffset, double num_exec); static Node *fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context); static List *set_returning_clause_references(PlannerInfo *root, @@ -177,17 +201,20 @@ static List *set_returning_clause_references(PlannerInfo *root, * 5. PARAM_MULTIEXPR Params are replaced by regular PARAM_EXEC Params, * now that we have finished planning all MULTIEXPR subplans. * - * 6. We compute regproc OIDs for operators (ie, we look up the function + * 6. AlternativeSubPlan expressions are replaced by just one of their + * alternatives, using an estimate of how many times they'll be executed. + * + * 7. We compute regproc OIDs for operators (ie, we look up the function * that implements each op). * - * 7. We create lists of specific objects that the plan depends on. + * 8. We create lists of specific objects that the plan depends on. * This will be used by plancache.c to drive invalidation of cached plans. * Relation dependencies are represented by OIDs, and everything else by * PlanInvalItems (this distinction is motivated by the shared-inval APIs). * Currently, relations, user-defined functions, and domains are the only * types of objects that are explicitly tracked this way. * - * 8. We assign every plan node in the tree a unique ID. + * 9. We assign every plan node in the tree a unique ID. * * We also perform one final optimization step, which is to delete * SubqueryScan, Append, and MergeAppend plan nodes that aren't doing @@ -490,9 +517,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scanrelid += rtoffset; splan->plan.targetlist = - fix_scan_list(root, splan->plan.targetlist, rtoffset); + fix_scan_list(root, splan->plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->plan.qual = - fix_scan_list(root, splan->plan.qual, rtoffset); + fix_scan_list(root, splan->plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_SampleScan: @@ -501,11 +530,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tablesample = (TableSampleClause *) - fix_scan_expr(root, (Node *) splan->tablesample, rtoffset); + fix_scan_expr(root, (Node *) splan->tablesample, + rtoffset, 1); } break; case T_IndexScan: @@ -514,17 +546,23 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->indexqual = - fix_scan_list(root, splan->indexqual, rtoffset); + fix_scan_list(root, splan->indexqual, + rtoffset, 1); splan->indexqualorig = - fix_scan_list(root, splan->indexqualorig, rtoffset); + fix_scan_list(root, splan->indexqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); splan->indexorderby = - fix_scan_list(root, splan->indexorderby, rtoffset); + fix_scan_list(root, splan->indexorderby, + rtoffset, 1); splan->indexorderbyorig = - fix_scan_list(root, splan->indexorderbyorig, rtoffset); + fix_scan_list(root, splan->indexorderbyorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_IndexOnlyScan: @@ -543,9 +581,10 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->scan.plan.targetlist == NIL); Assert(splan->scan.plan.qual == NIL); splan->indexqual = - fix_scan_list(root, splan->indexqual, rtoffset); + fix_scan_list(root, splan->indexqual, rtoffset, 1); splan->indexqualorig = - fix_scan_list(root, splan->indexqualorig, rtoffset); + fix_scan_list(root, splan->indexqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_BitmapHeapScan: @@ -554,11 +593,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->bitmapqualorig = - fix_scan_list(root, splan->bitmapqualorig, rtoffset); + fix_scan_list(root, splan->bitmapqualorig, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_TidScan: @@ -567,11 +609,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tidquals = - fix_scan_list(root, splan->tidquals, rtoffset); + fix_scan_list(root, splan->tidquals, + rtoffset, 1); } break; case T_SubqueryScan: @@ -585,11 +630,13 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->functions = - fix_scan_list(root, splan->functions, rtoffset); + fix_scan_list(root, splan->functions, rtoffset, 1); } break; case T_TableFuncScan: @@ -598,11 +645,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->tablefunc = (TableFunc *) - fix_scan_expr(root, (Node *) splan->tablefunc, rtoffset); + fix_scan_expr(root, (Node *) splan->tablefunc, + rtoffset, 1); } break; case T_ValuesScan: @@ -611,11 +661,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); splan->values_lists = - fix_scan_list(root, splan->values_lists, rtoffset); + fix_scan_list(root, splan->values_lists, + rtoffset, 1); } break; case T_CteScan: @@ -624,9 +677,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_NamedTuplestoreScan: @@ -635,9 +690,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_WorkTableScan: @@ -646,9 +703,11 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->scan.scanrelid += rtoffset; splan->scan.plan.targetlist = - fix_scan_list(root, splan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, splan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->scan.plan.qual = - fix_scan_list(root, splan->scan.plan.qual, rtoffset); + fix_scan_list(root, splan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } break; case T_ForeignScan: @@ -732,9 +791,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->plan.qual == NIL); splan->limitOffset = - fix_scan_expr(root, splan->limitOffset, rtoffset); + fix_scan_expr(root, splan->limitOffset, rtoffset, 1); splan->limitCount = - fix_scan_expr(root, splan->limitCount, rtoffset); + fix_scan_expr(root, splan->limitCount, rtoffset, 1); } break; case T_Agg: @@ -775,9 +834,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) * variable refs, so fix_scan_expr works for them. */ wplan->startOffset = - fix_scan_expr(root, wplan->startOffset, rtoffset); + fix_scan_expr(root, wplan->startOffset, rtoffset, 1); wplan->endOffset = - fix_scan_expr(root, wplan->endOffset, rtoffset); + fix_scan_expr(root, wplan->endOffset, rtoffset, 1); } break; case T_Result: @@ -793,13 +852,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) else { splan->plan.targetlist = - fix_scan_list(root, splan->plan.targetlist, rtoffset); + fix_scan_list(root, splan->plan.targetlist, + rtoffset, NUM_EXEC_TLIST(plan)); splan->plan.qual = - fix_scan_list(root, splan->plan.qual, rtoffset); + fix_scan_list(root, splan->plan.qual, + rtoffset, NUM_EXEC_QUAL(plan)); } /* resconstantqual can't contain any subplan variable refs */ splan->resconstantqual = - fix_scan_expr(root, splan->resconstantqual, rtoffset); + fix_scan_expr(root, splan->resconstantqual, rtoffset, 1); } break; case T_ProjectSet: @@ -813,7 +874,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) Assert(splan->plan.qual == NIL); splan->withCheckOptionLists = - fix_scan_list(root, splan->withCheckOptionLists, rtoffset); + fix_scan_list(root, splan->withCheckOptionLists, + rtoffset, 1); if (splan->returningLists) { @@ -874,18 +936,18 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) fix_join_expr(root, splan->onConflictSet, NULL, itlist, linitial_int(splan->resultRelations), - rtoffset); + rtoffset, NUM_EXEC_QUAL(plan)); splan->onConflictWhere = (Node *) fix_join_expr(root, (List *) splan->onConflictWhere, NULL, itlist, linitial_int(splan->resultRelations), - rtoffset); + rtoffset, NUM_EXEC_QUAL(plan)); pfree(itlist); splan->exclRelTlist = - fix_scan_list(root, splan->exclRelTlist, rtoffset); + fix_scan_list(root, splan->exclRelTlist, rtoffset, 1); } splan->nominalRelation += rtoffset; @@ -1026,19 +1088,24 @@ set_indexonlyscan_references(PlannerInfo *root, (Node *) plan->scan.plan.targetlist, index_itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) plan)); plan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) plan->scan.plan.qual, index_itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) plan)); /* indexqual is already transformed to reference index columns */ - plan->indexqual = fix_scan_list(root, plan->indexqual, rtoffset); + plan->indexqual = fix_scan_list(root, plan->indexqual, + rtoffset, 1); /* indexorderby is already transformed to reference index columns */ - plan->indexorderby = fix_scan_list(root, plan->indexorderby, rtoffset); + plan->indexorderby = fix_scan_list(root, plan->indexorderby, + rtoffset, 1); /* indextlist must NOT be transformed to reference index columns */ - plan->indextlist = fix_scan_list(root, plan->indextlist, rtoffset); + plan->indextlist = fix_scan_list(root, plan->indextlist, + rtoffset, NUM_EXEC_TLIST((Plan *) plan)); pfree(index_itlist); @@ -1084,9 +1151,11 @@ set_subqueryscan_references(PlannerInfo *root, */ plan->scan.scanrelid += rtoffset; plan->scan.plan.targetlist = - fix_scan_list(root, plan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, plan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) plan)); plan->scan.plan.qual = - fix_scan_list(root, plan->scan.plan.qual, rtoffset); + fix_scan_list(root, plan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) plan)); result = (Plan *) plan; } @@ -1202,29 +1271,34 @@ set_foreignscan_references(PlannerInfo *root, (Node *) fscan->scan.plan.targetlist, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) fscan)); fscan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) fscan->scan.plan.qual, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_exprs = (List *) fix_upper_expr(root, (Node *) fscan->fdw_exprs, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_recheck_quals = (List *) fix_upper_expr(root, (Node *) fscan->fdw_recheck_quals, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) fscan)); pfree(itlist); /* fdw_scan_tlist itself just needs fix_scan_list() adjustments */ fscan->fdw_scan_tlist = - fix_scan_list(root, fscan->fdw_scan_tlist, rtoffset); + fix_scan_list(root, fscan->fdw_scan_tlist, + rtoffset, NUM_EXEC_TLIST((Plan *) fscan)); } else { @@ -1233,13 +1307,17 @@ set_foreignscan_references(PlannerInfo *root, * way */ fscan->scan.plan.targetlist = - fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, fscan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) fscan)); fscan->scan.plan.qual = - fix_scan_list(root, fscan->scan.plan.qual, rtoffset); + fix_scan_list(root, fscan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_exprs = - fix_scan_list(root, fscan->fdw_exprs, rtoffset); + fix_scan_list(root, fscan->fdw_exprs, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); fscan->fdw_recheck_quals = - fix_scan_list(root, fscan->fdw_recheck_quals, rtoffset); + fix_scan_list(root, fscan->fdw_recheck_quals, + rtoffset, NUM_EXEC_QUAL((Plan *) fscan)); } fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset); @@ -1270,33 +1348,40 @@ set_customscan_references(PlannerInfo *root, (Node *) cscan->scan.plan.targetlist, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) cscan)); cscan->scan.plan.qual = (List *) fix_upper_expr(root, (Node *) cscan->scan.plan.qual, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) cscan)); cscan->custom_exprs = (List *) fix_upper_expr(root, (Node *) cscan->custom_exprs, itlist, INDEX_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) cscan)); pfree(itlist); /* custom_scan_tlist itself just needs fix_scan_list() adjustments */ cscan->custom_scan_tlist = - fix_scan_list(root, cscan->custom_scan_tlist, rtoffset); + fix_scan_list(root, cscan->custom_scan_tlist, + rtoffset, NUM_EXEC_TLIST((Plan *) cscan)); } else { /* Adjust tlist, qual, custom_exprs in the standard way */ cscan->scan.plan.targetlist = - fix_scan_list(root, cscan->scan.plan.targetlist, rtoffset); + fix_scan_list(root, cscan->scan.plan.targetlist, + rtoffset, NUM_EXEC_TLIST((Plan *) cscan)); cscan->scan.plan.qual = - fix_scan_list(root, cscan->scan.plan.qual, rtoffset); + fix_scan_list(root, cscan->scan.plan.qual, + rtoffset, NUM_EXEC_QUAL((Plan *) cscan)); cscan->custom_exprs = - fix_scan_list(root, cscan->custom_exprs, rtoffset); + fix_scan_list(root, cscan->custom_exprs, + rtoffset, NUM_EXEC_QUAL((Plan *) cscan)); } /* Adjust child plan-nodes recursively, if needed */ @@ -1458,7 +1543,8 @@ set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) hplan->hashkeys, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL(plan)); /* Hash doesn't project */ set_dummy_tlist_references(plan, rtoffset); @@ -1623,6 +1709,53 @@ fix_param_node(PlannerInfo *root, Param *p) return (Node *) copyObject(p); } +/* + * fix_alternative_subplan + * Do set_plan_references processing on an AlternativeSubPlan + * + * Choose one of the alternative implementations and return just that one, + * discarding the rest of the AlternativeSubPlan structure. + * Note: caller must still recurse into the result! + * + * We don't make any attempt to fix up cost estimates in the parent plan + * node or higher-level nodes. However, we do delete the rejected subplan + * from root->glob->subplans, to minimize cycles expended on it later. + */ +static Node * +fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan, + double num_exec) +{ + SubPlan *subplan1; + SubPlan *subplan2; + Cost cost1; + Cost cost2; + ListCell *lc; + + /* Currently there will always be exactly 2 alternatives */ + Assert(list_length(asplan->subplans) == 2); + subplan1 = (SubPlan *) linitial(asplan->subplans); + subplan2 = (SubPlan *) lsecond(asplan->subplans); + + /* Choose the one that's cheaper assuming num_exec executions */ + cost1 = subplan1->startup_cost + num_exec * subplan1->per_call_cost; + cost2 = subplan2->startup_cost + num_exec * subplan2->per_call_cost; + + if (cost1 < cost2) + { + /* Delete subplan2 and return subplan1 */ + lc = list_nth_cell(root->glob->subplans, subplan2->plan_id - 1); + lfirst(lc) = NULL; + return (Node *) subplan1; + } + else + { + /* Delete subplan1 and return subplan2 */ + lc = list_nth_cell(root->glob->subplans, subplan1->plan_id - 1); + lfirst(lc) = NULL; + return (Node *) subplan2; + } +} + /* * fix_scan_expr * Do set_plan_references processing on a scan-level expression @@ -1630,21 +1763,24 @@ fix_param_node(PlannerInfo *root, Param *p) * This consists of incrementing all Vars' varnos by rtoffset, * replacing PARAM_MULTIEXPR Params, expanding PlaceHolderVars, * replacing Aggref nodes that should be replaced by initplan output Params, + * choosing the best implementation for AlternativeSubPlans, * looking up operator opcode info for OpExpr and related nodes, * and adding OIDs from regclass Const nodes into root->glob->relationOids. */ static Node * -fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset) +fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset, double num_exec) { fix_scan_expr_context context; context.root = root; context.rtoffset = rtoffset; + context.num_exec = num_exec; if (rtoffset != 0 || root->multiexpr_params != NIL || root->glob->lastPHId != 0 || - root->minmax_aggs != NIL) + root->minmax_aggs != NIL || + root->hasAlternativeSubPlans) { return fix_scan_expr_mutator(node, &context); } @@ -1655,7 +1791,8 @@ fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset) * are no MULTIEXPR subqueries then we don't need to replace * PARAM_MULTIEXPR Params, and if there are no placeholders anywhere * we won't need to remove them, and if there are no minmax Aggrefs we - * won't need to replace them. Then it's OK to just scribble on the + * won't need to replace them, and if there are no AlternativeSubPlans + * we won't need to remove them. Then it's OK to just scribble on the * input node tree instead of copying (since the only change, filling * in any unset opfuncid fields, is harmless). This saves just enough * cycles to be noticeable on trivial queries. @@ -1729,6 +1866,11 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context) return fix_scan_expr_mutator((Node *) phv->phexpr, context); } + if (IsA(node, AlternativeSubPlan)) + return fix_scan_expr_mutator(fix_alternative_subplan(context->root, + (AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_scan_expr_mutator, (void *) context); @@ -1740,6 +1882,7 @@ fix_scan_expr_walker(Node *node, fix_scan_expr_context *context) if (node == NULL) return false; Assert(!IsA(node, PlaceHolderVar)); + Assert(!IsA(node, AlternativeSubPlan)); fix_expr_common(context->root, node); return expression_tree_walker(node, fix_scan_expr_walker, (void *) context); @@ -1776,7 +1919,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); /* Now do join-type-specific stuff */ if (IsA(join, NestLoop)) @@ -1792,7 +1936,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) (Node *) nlp->paramval, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(outer_plan)); /* Check we replaced any PlaceHolderVar with simple Var */ if (!(IsA(nlp->paramval, Var) && nlp->paramval->varno == OUTER_VAR)) @@ -1808,7 +1953,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); } else if (IsA(join, HashJoin)) { @@ -1819,7 +1965,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); /* * HashJoin's hashkeys are used to look for matching tuples from its @@ -1829,7 +1976,8 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) (Node *) hj->hashkeys, outer_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); } /* @@ -1867,13 +2015,15 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset) outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_TLIST((Plan *) join)); join->plan.qual = fix_join_expr(root, join->plan.qual, outer_itlist, inner_itlist, (Index) 0, - rtoffset); + rtoffset, + NUM_EXEC_QUAL((Plan *) join)); pfree(outer_itlist); pfree(inner_itlist); @@ -1926,14 +2076,16 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) tle->expr, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(plan)); } else newexpr = fix_upper_expr(root, (Node *) tle->expr, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(plan)); tle = flatCopyTargetEntry(tle); tle->expr = (Expr *) newexpr; output_targetlist = lappend(output_targetlist, tle); @@ -1945,7 +2097,8 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) (Node *) plan->qual, subplan_itlist, OUTER_VAR, - rtoffset); + rtoffset, + NUM_EXEC_QUAL(plan)); pfree(subplan_itlist); } @@ -2389,6 +2542,7 @@ search_indexed_tlist_for_sortgroupref(Expr *node, * 'acceptable_rel' is either zero or the rangetable index of a relation * whose Vars may appear in the clause without provoking an error * 'rtoffset': how much to increment varnos by + * 'num_exec': estimated number of executions of expression * * Returns the new expression tree. The original clause structure is * not modified. @@ -2399,7 +2553,8 @@ fix_join_expr(PlannerInfo *root, indexed_tlist *outer_itlist, indexed_tlist *inner_itlist, Index acceptable_rel, - int rtoffset) + int rtoffset, + double num_exec) { fix_join_expr_context context; @@ -2408,6 +2563,7 @@ fix_join_expr(PlannerInfo *root, context.inner_itlist = inner_itlist; context.acceptable_rel = acceptable_rel; context.rtoffset = rtoffset; + context.num_exec = num_exec; return (List *) fix_join_expr_mutator((Node *) clauses, &context); } @@ -2502,6 +2658,11 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) /* Special cases (apply only AFTER failing to match to lower tlist) */ if (IsA(node, Param)) return fix_param_node(context->root, (Param *) node); + if (IsA(node, AlternativeSubPlan)) + return fix_join_expr_mutator(fix_alternative_subplan(context->root, + (AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_join_expr_mutator, @@ -2533,6 +2694,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) * 'subplan_itlist': indexed target list for subplan (or index) * 'newvarno': varno to use for Vars referencing tlist elements * 'rtoffset': how much to increment varnos by + * 'num_exec': estimated number of executions of expression * * The resulting tree is a copy of the original in which all Var nodes have * varno = newvarno, varattno = resno of corresponding targetlist element. @@ -2543,7 +2705,8 @@ fix_upper_expr(PlannerInfo *root, Node *node, indexed_tlist *subplan_itlist, Index newvarno, - int rtoffset) + int rtoffset, + double num_exec) { fix_upper_expr_context context; @@ -2551,6 +2714,7 @@ fix_upper_expr(PlannerInfo *root, context.subplan_itlist = subplan_itlist; context.newvarno = newvarno; context.rtoffset = rtoffset; + context.num_exec = num_exec; return fix_upper_expr_mutator(node, &context); } @@ -2623,6 +2787,11 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context) } /* If no match, just fall through to process it normally */ } + if (IsA(node, AlternativeSubPlan)) + return fix_upper_expr_mutator(fix_alternative_subplan(context->root, + (AlternativeSubPlan *) node, + context->num_exec), + context); fix_expr_common(context->root, node); return expression_tree_mutator(node, fix_upper_expr_mutator, @@ -2687,7 +2856,8 @@ set_returning_clause_references(PlannerInfo *root, itlist, NULL, resultRelation, - rtoffset); + rtoffset, + NUM_EXEC_TLIST(topplan)); pfree(itlist); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 6eb794669f..fcce81926b 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -81,6 +81,7 @@ static Node *convert_testexpr(PlannerInfo *root, static Node *convert_testexpr_mutator(Node *node, convert_testexpr_context *context); static bool subplan_is_hashable(Plan *plan); +static bool subpath_is_hashable(Path *path); static bool testexpr_is_hashable(Node *testexpr, List *param_ids); static bool test_opexpr_is_hashable(OpExpr *testexpr, List *param_ids); static bool hash_ok_operator(OpExpr *expr); @@ -247,7 +248,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, * likely to be better (it depends on the expected number of executions of * the EXISTS qual, and we are much too early in planning the outer query * to be able to guess that). So we generate both plans, if possible, and - * leave it to the executor to decide which to use. + * leave it to setrefs.c to decide which to use. */ if (simple_exists && IsA(result, SubPlan)) { @@ -273,20 +274,20 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, plan_params = root->plan_params; root->plan_params = NIL; - /* Select best Path and turn it into a Plan */ + /* Select best Path */ final_rel = fetch_upper_rel(subroot, UPPERREL_FINAL, NULL); best_path = final_rel->cheapest_total_path; - plan = create_plan(subroot, best_path); - /* Now we can check if it'll fit in hash_mem */ - /* XXX can we check this at the Path stage? */ - if (subplan_is_hashable(plan)) + if (subpath_is_hashable(best_path)) { SubPlan *hashplan; AlternativeSubPlan *asplan; - /* OK, convert to SubPlan format. */ + /* OK, finish planning the ANY subquery */ + plan = create_plan(subroot, best_path); + + /* ... and convert to SubPlan format */ hashplan = castNode(SubPlan, build_subplan(root, plan, subroot, plan_params, @@ -298,10 +299,11 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, Assert(hashplan->parParam == NIL); Assert(hashplan->useHashTable); - /* Leave it to the executor to decide which plan to use */ + /* Leave it to setrefs.c to decide which plan to use */ asplan = makeNode(AlternativeSubPlan); asplan->subplans = list_make2(result, hashplan); result = (Node *) asplan; + root->hasAlternativeSubPlans = true; } } } @@ -714,6 +716,9 @@ convert_testexpr_mutator(Node *node, /* * subplan_is_hashable: can we implement an ANY subplan by hashing? + * + * This is not responsible for checking whether the combining testexpr + * is suitable for hashing. We only look at the subquery itself. */ static bool subplan_is_hashable(Plan *plan) @@ -735,6 +740,31 @@ subplan_is_hashable(Plan *plan) return true; } +/* + * subpath_is_hashable: can we implement an ANY subplan by hashing? + * + * Identical to subplan_is_hashable, but work from a Path for the subplan. + */ +static bool +subpath_is_hashable(Path *path) +{ + double subquery_size; + int hash_mem = get_hash_mem(); + + /* + * The estimated size of the subquery result must fit in hash_mem. (Note: + * we use heap tuple overhead here even though the tuples will actually be + * stored as MinimalTuples; this provides some fudge factor for hashtable + * overhead.) + */ + subquery_size = path->rows * + (MAXALIGN(path->pathtarget->width) + MAXALIGN(SizeofHeapTupleHeader)); + if (subquery_size > hash_mem * 1024L) + return false; + + return true; +} + /* * testexpr_is_hashable: is an ANY SubLink's test expression hashable? * diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 485d1b06c9..dbe86e7af6 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -347,6 +347,7 @@ struct PlannerInfo bool hasHavingQual; /* true if havingQual was non-null */ bool hasPseudoConstantQuals; /* true if any RestrictInfo has * pseudoconstant = true */ + bool hasAlternativeSubPlans; /* true if we've made any of those */ bool hasRecursion; /* true if planning a recursive WITH item */ /* These fields are used only when hasRecursion is true: */ diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index d73be2ad46..fd65ee8f9c 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -736,6 +736,9 @@ typedef struct SubPlan /* * AlternativeSubPlan - expression node for a choice among SubPlans * + * This is used only transiently during planning: by the time the plan + * reaches the executor, all AlternativeSubPlan nodes have been removed. + * * The subplans are given as a List so that the node definition need not * change if there's ever more than two alternatives. For the moment, * though, there are always exactly two; and the first one is the fast-start diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 1338b2b23e..ff157ceb1c 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -50,14 +50,12 @@ explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on con Insert on insertconflicttest Conflict Resolution: UPDATE Conflict Arbiter Indexes: op_index_key, collation_index_key, both_index_key - Conflict Filter: (alternatives: SubPlan 1 or hashed SubPlan 2) + Conflict Filter: (SubPlan 1) -> Result SubPlan 1 -> Index Only Scan using both_index_expr_key on insertconflicttest ii Index Cond: (key = excluded.key) - SubPlan 2 - -> Seq Scan on insertconflicttest ii_1 -(10 rows) +(8 rows) -- Neither collation nor operator class specifications are required -- -- supplying them merely *limits* matches to indexes with matching opclasses diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index b81923f2e7..9d56cdacf3 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -874,6 +874,53 @@ select * from int8_tbl where q1 in (select c1 from inner_text); (2 rows) rollback; -- to get rid of the bogus operator +-- +-- Test resolution of hashed vs non-hashed implementation of EXISTS subplan +-- +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + QUERY PLAN +-------------------------------------------------------------- + Aggregate + -> Seq Scan on tenk1 t + Filter: ((hashed SubPlan 2) OR (ten < 0)) + SubPlan 2 + -> Index Only Scan using tenk1_unique1 on tenk1 k +(5 rows) + +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + count +------- + 10000 +(1 row) + +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + QUERY PLAN +-------------------------------------------------------------- + Aggregate + -> Bitmap Heap Scan on tenk1 t + Recheck Cond: (thousand = 1) + Filter: ((SubPlan 1) OR (ten < 0)) + -> Bitmap Index Scan on tenk1_thous_tenthous + Index Cond: (thousand = 1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on tenk1 k + Index Cond: (unique1 = t.unique2) +(9 rows) + +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + count +------- + 10 +(1 row) + -- -- Test case for planner bug with nested EXISTS handling -- diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 5de53f2782..caed1c19ec 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -1869,9 +1869,7 @@ EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (5); SubPlan 1 -> Index Only Scan using ref_tbl_pkey on ref_tbl r Index Cond: (a = b.a) - SubPlan 2 - -> Seq Scan on ref_tbl r_1 -(7 rows) +(5 rows) EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; QUERY PLAN @@ -1885,9 +1883,7 @@ EXPLAIN (costs off) UPDATE rw_view1 SET a = a + 5; SubPlan 1 -> Index Only Scan using ref_tbl_pkey on ref_tbl r_1 Index Cond: (a = b.a) - SubPlan 2 - -> Seq Scan on ref_tbl r_2 -(11 rows) +(9 rows) DROP TABLE base_tbl, ref_tbl CASCADE; NOTICE: drop cascades to view rw_view1 @@ -2301,8 +2297,8 @@ SELECT * FROM v1 WHERE a=8; EXPLAIN (VERBOSE, COSTS OFF) UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; - QUERY PLAN ---------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------------------------------- Update on public.t1 Update on public.t1 Update on public.t11 t1_1 @@ -2311,32 +2307,26 @@ UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; -> Index Scan using t1_a_idx on public.t1 Output: 100, t1.b, t1.c, t1.ctid Index Cond: ((t1.a > 5) AND (t1.a < 7)) - Filter: ((t1.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1.a) AND leakproof(t1.a)) + Filter: ((t1.a <> 6) AND (SubPlan 1) AND snoop(t1.a) AND leakproof(t1.a)) SubPlan 1 -> Append -> Seq Scan on public.t12 t12_1 Filter: (t12_1.a = t1.a) -> Seq Scan on public.t111 t12_2 Filter: (t12_2.a = t1.a) - SubPlan 2 - -> Append - -> Seq Scan on public.t12 t12_4 - Output: t12_4.a - -> Seq Scan on public.t111 t12_5 - Output: t12_5.a -> Index Scan using t11_a_idx on public.t11 t1_1 Output: 100, t1_1.b, t1_1.c, t1_1.d, t1_1.ctid Index Cond: ((t1_1.a > 5) AND (t1_1.a < 7)) - Filter: ((t1_1.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_1.a) AND leakproof(t1_1.a)) + Filter: ((t1_1.a <> 6) AND (SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a)) -> Index Scan using t12_a_idx on public.t12 t1_2 Output: 100, t1_2.b, t1_2.c, t1_2.e, t1_2.ctid Index Cond: ((t1_2.a > 5) AND (t1_2.a < 7)) - Filter: ((t1_2.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_2.a) AND leakproof(t1_2.a)) + Filter: ((t1_2.a <> 6) AND (SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a)) -> Index Scan using t111_a_idx on public.t111 t1_3 Output: 100, t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid Index Cond: ((t1_3.a > 5) AND (t1_3.a < 7)) - Filter: ((t1_3.a <> 6) AND (alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_3.a) AND leakproof(t1_3.a)) -(33 rows) + Filter: ((t1_3.a <> 6) AND (SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a)) +(27 rows) UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6; SELECT * FROM v1 WHERE a=100; -- Nothing should have been changed to 100 @@ -2351,8 +2341,8 @@ SELECT * FROM t1 WHERE a=100; -- Nothing should have been changed to 100 EXPLAIN (VERBOSE, COSTS OFF) UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; - QUERY PLAN ---------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------- Update on public.t1 Update on public.t1 Update on public.t11 t1_1 @@ -2361,32 +2351,26 @@ UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; -> Index Scan using t1_a_idx on public.t1 Output: (t1.a + 1), t1.b, t1.c, t1.ctid Index Cond: ((t1.a > 5) AND (t1.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1.a) AND leakproof(t1.a)) + Filter: ((SubPlan 1) AND snoop(t1.a) AND leakproof(t1.a)) SubPlan 1 -> Append -> Seq Scan on public.t12 t12_1 Filter: (t12_1.a = t1.a) -> Seq Scan on public.t111 t12_2 Filter: (t12_2.a = t1.a) - SubPlan 2 - -> Append - -> Seq Scan on public.t12 t12_4 - Output: t12_4.a - -> Seq Scan on public.t111 t12_5 - Output: t12_5.a -> Index Scan using t11_a_idx on public.t11 t1_1 Output: (t1_1.a + 1), t1_1.b, t1_1.c, t1_1.d, t1_1.ctid Index Cond: ((t1_1.a > 5) AND (t1_1.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_1.a) AND leakproof(t1_1.a)) + Filter: ((SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a)) -> Index Scan using t12_a_idx on public.t12 t1_2 Output: (t1_2.a + 1), t1_2.b, t1_2.c, t1_2.e, t1_2.ctid Index Cond: ((t1_2.a > 5) AND (t1_2.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_2.a) AND leakproof(t1_2.a)) + Filter: ((SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a)) -> Index Scan using t111_a_idx on public.t111 t1_3 Output: (t1_3.a + 1), t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid Index Cond: ((t1_3.a > 5) AND (t1_3.a = 8)) - Filter: ((alternatives: SubPlan 1 or hashed SubPlan 2) AND snoop(t1_3.a) AND leakproof(t1_3.a)) -(33 rows) + Filter: ((SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a)) +(27 rows) UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8; NOTICE: snooped value: 8 diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index cce8ebdb3d..a25cb6fc5c 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -509,6 +509,23 @@ select * from int8_tbl where q1 in (select c1 from inner_text); rollback; -- to get rid of the bogus operator +-- +-- Test resolution of hashed vs non-hashed implementation of EXISTS subplan +-- +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0); + +explain (costs off) +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; +select count(*) from tenk1 t +where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0) + and thousand = 1; + -- -- Test case for planner bug with nested EXISTS handling -- diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 236413f62a..868f8b0858 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1104,23 +1104,6 @@ ExecInitExprRec(Expr *node, ExprState *state, break; } - case T_AlternativeSubPlan: - { - AlternativeSubPlan *asplan = (AlternativeSubPlan *) node; - AlternativeSubPlanState *asstate; - - if (!state->parent) - elog(ERROR, "AlternativeSubPlan found with no parent plan"); - - asstate = ExecInitAlternativeSubPlan(asplan, state->parent); - - scratch.opcode = EEOP_ALTERNATIVE_SUBPLAN; - scratch.d.alternative_subplan.asstate = asstate; - - ExprEvalPushStep(state, &scratch); - break; - } - case T_FieldSelect: { FieldSelect *fselect = (FieldSelect *) node; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index b812bbacee..26c2b49632 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -431,7 +431,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_GROUPING_FUNC, &&CASE_EEOP_WINDOW_FUNC, &&CASE_EEOP_SUBPLAN, - &&CASE_EEOP_ALTERNATIVE_SUBPLAN, &&CASE_EEOP_AGG_STRICT_DESERIALIZE, &&CASE_EEOP_AGG_DESERIALIZE, &&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS, @@ -1536,14 +1535,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } - EEO_CASE(EEOP_ALTERNATIVE_SUBPLAN) - { - /* too complex for an inline implementation */ - ExecEvalAlternativeSubPlan(state, op, econtext); - - EEO_NEXT(); - } - /* evaluate a strict aggregate deserialization function */ EEO_CASE(EEOP_AGG_STRICT_DESERIALIZE) { @@ -3868,20 +3859,6 @@ ExecEvalSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext) *op->resvalue = ExecSubPlan(sstate, econtext, op->resnull); } -/* - * Hand off evaluation of an alternative subplan to nodeSubplan.c - */ -void -ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext) -{ - AlternativeSubPlanState *asstate = op->d.alternative_subplan.asstate; - - /* could potentially be nested, so make sure there's enough stack */ - check_stack_depth(); - - *op->resvalue = ExecAlternativeSubPlan(asstate, econtext, op->resnull); -} - /* * Evaluate a wholerow Var expression. * diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 9a7962518e..9a706df5f0 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -1303,83 +1303,3 @@ ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent) parent->chgParam = bms_add_member(parent->chgParam, paramid); } } - - -/* - * ExecInitAlternativeSubPlan - * - * Initialize for execution of one of a set of alternative subplans. - */ -AlternativeSubPlanState * -ExecInitAlternativeSubPlan(AlternativeSubPlan *asplan, PlanState *parent) -{ - AlternativeSubPlanState *asstate = makeNode(AlternativeSubPlanState); - double num_calls; - SubPlan *subplan1; - SubPlan *subplan2; - Cost cost1; - Cost cost2; - ListCell *lc; - - asstate->subplan = asplan; - - /* - * Initialize subplans. (Can we get away with only initializing the one - * we're going to use?) - */ - foreach(lc, asplan->subplans) - { - SubPlan *sp = lfirst_node(SubPlan, lc); - SubPlanState *sps = ExecInitSubPlan(sp, parent); - - asstate->subplans = lappend(asstate->subplans, sps); - parent->subPlan = lappend(parent->subPlan, sps); - } - - /* - * Select the one to be used. For this, we need an estimate of the number - * of executions of the subplan. We use the number of output rows - * expected from the parent plan node. This is a good estimate if we are - * in the parent's targetlist, and an underestimate (but probably not by - * more than a factor of 2) if we are in the qual. - */ - num_calls = parent->plan->plan_rows; - - /* - * The planner saved enough info so that we don't have to work very hard - * to estimate the total cost, given the number-of-calls estimate. - */ - Assert(list_length(asplan->subplans) == 2); - subplan1 = (SubPlan *) linitial(asplan->subplans); - subplan2 = (SubPlan *) lsecond(asplan->subplans); - - cost1 = subplan1->startup_cost + num_calls * subplan1->per_call_cost; - cost2 = subplan2->startup_cost + num_calls * subplan2->per_call_cost; - - if (cost1 < cost2) - asstate->active = 0; - else - asstate->active = 1; - - return asstate; -} - -/* - * ExecAlternativeSubPlan - * - * Execute one of a set of alternative subplans. - * - * Note: in future we might consider changing to different subplans on the - * fly, in case the original rowcount estimate turns out to be way off. - */ -Datum -ExecAlternativeSubPlan(AlternativeSubPlanState *node, - ExprContext *econtext, - bool *isNull) -{ - /* Just pass control to the active subplan */ - SubPlanState *activesp = list_nth_node(SubPlanState, - node->subplans, node->active); - - return ExecSubPlan(activesp, econtext, isNull); -} diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index cca5c117a0..eb1dea658c 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1918,12 +1918,6 @@ llvm_compile_expr(ExprState *state) LLVMBuildBr(b, opblocks[opno + 1]); break; - case EEOP_ALTERNATIVE_SUBPLAN: - build_EvalXFunc(b, mod, "ExecEvalAlternativeSubPlan", - v_state, op, v_econtext); - LLVMBuildBr(b, opblocks[opno + 1]); - break; - case EEOP_AGG_STRICT_DESERIALIZE: case EEOP_AGG_DESERIALIZE: { diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index 0a93d5f665..1ed3cafa2f 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -102,7 +102,6 @@ void *referenced_functions[] = ExecAggTransReparent, ExecEvalAggOrderedTransDatum, ExecEvalAggOrderedTransTuple, - ExecEvalAlternativeSubPlan, ExecEvalArrayCoerce, ExecEvalArrayExpr, ExecEvalConstraintCheck, diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 60dd80c23c..0e3084325e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8192,7 +8192,12 @@ get_rule_expr(Node *node, deparse_context *context, AlternativeSubPlan *asplan = (AlternativeSubPlan *) node; ListCell *lc; - /* As above, this can only happen during EXPLAIN */ + /* + * This case cannot be reached in normal usage, since no + * AlternativeSubPlan can appear either in parsetrees or + * finished plan trees. We keep it just in case somebody + * wants to use this code to print planner data structures. + */ appendStringInfoString(buf, "(alternatives: "); foreach(lc, asplan->subplans) { diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index dbe8649a57..b792de1bc9 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -218,7 +218,6 @@ typedef enum ExprEvalOp EEOP_GROUPING_FUNC, EEOP_WINDOW_FUNC, EEOP_SUBPLAN, - EEOP_ALTERNATIVE_SUBPLAN, /* aggregation related nodes */ EEOP_AGG_STRICT_DESERIALIZE, @@ -589,13 +588,6 @@ typedef struct ExprEvalStep SubPlanState *sstate; } subplan; - /* for EEOP_ALTERNATIVE_SUBPLAN */ - struct - { - /* out-of-line state, created by nodeSubplan.c */ - AlternativeSubPlanState *asstate; - } alternative_subplan; - /* for EEOP_AGG_*DESERIALIZE */ struct { @@ -734,8 +726,6 @@ extern void ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op); extern void ExecEvalGroupingFunc(ExprState *state, ExprEvalStep *op); extern void ExecEvalSubPlan(ExprState *state, ExprEvalStep *op, ExprContext *econtext); -extern void ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op, - ExprContext *econtext); extern void ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalSysVar(ExprState *state, ExprEvalStep *op, diff --git a/src/include/executor/nodeSubplan.h b/src/include/executor/nodeSubplan.h index 83e90b3d07..b629af1f5f 100644 --- a/src/include/executor/nodeSubplan.h +++ b/src/include/executor/nodeSubplan.h @@ -18,12 +18,8 @@ extern SubPlanState *ExecInitSubPlan(SubPlan *subplan, PlanState *parent); -extern AlternativeSubPlanState *ExecInitAlternativeSubPlan(AlternativeSubPlan *asplan, PlanState *parent); - extern Datum ExecSubPlan(SubPlanState *node, ExprContext *econtext, bool *isNull); -extern Datum ExecAlternativeSubPlan(AlternativeSubPlanState *node, ExprContext *econtext, bool *isNull); - extern void ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent); extern void ExecSetParamPlan(SubPlanState *node, ExprContext *econtext); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 0b42dd6f94..7e02afa6ba 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -880,18 +880,6 @@ typedef struct SubPlanState ExprState *cur_eq_comp; /* equality comparator for LHS vs. table */ } SubPlanState; -/* ---------------- - * AlternativeSubPlanState node - * ---------------- - */ -typedef struct AlternativeSubPlanState -{ - NodeTag type; - AlternativeSubPlan *subplan; /* expression plan node */ - List *subplans; /* SubPlanStates of alternative subplans */ - int active; /* list index of the one we're using */ -} AlternativeSubPlanState; - /* * DomainConstraintState - one item to check during CoerceToDomain * diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index 381d84b4e4..7ddd8c011b 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -213,7 +213,6 @@ typedef enum NodeTag T_WindowFuncExprState, T_SetExprState, T_SubPlanState, - T_AlternativeSubPlanState, T_DomainConstraintState, /*
Andy Fan <zhihui.fan1213@gmail.com> writes: > Thank you for this code! I still have some confusion about when a SubPlan > should be executed when a join is involved. I care about this because this > has an impact on when we can get the num_exec for a subplan. > The subplan in a target list, it is executed after the join in my case. > The subplan > can be execute after the scan of T1(see below example) and it can also be > executed > after the join. Which one is better depends on which methods make the > num_exec > smaller. Is it something we already considered? Uh, I'm not following your concern. SubPlans appearing in the join targetlist *must* be executed "after the join", ie only for valid join rows. Otherwise we could have cases where, say, they throw errors that should not occur. On the other hand, SubPlans appearing in the join's qual conditions have to be executed "before the join", although exactly what that means is fuzzy because we don't make any promises about the relative ordering of different qual conditions. > When the subplan is in a Qual, it is supposed to be executed as soon as > possible, > The current implementation matches the below cases. So can we say we > knows the num_execs of SubPlan just after we plan the dependent rels? I wouldn't say so. If the SubPlan's qual actually only depends on one of the input rels, distribute_qual_to_rels would have pushed it down further than the join. Among the quals that do have to be evaluated at the join, a qual involving a SubPlan is best executed last on cost grounds, or so I'd guess anyway. So the number of executions is probably less than the product of the input rel sizes. That's what motivates the choice of NUM_EXEC_QUAL in my patch. regards, tom lane
On Tue, Sep 1, 2020 at 1:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andy Fan <zhihui.fan1213@gmail.com> writes:
> Thank you for this code! I still have some confusion about when a SubPlan
> should be executed when a join is involved. I care about this because this
> has an impact on when we can get the num_exec for a subplan.
> The subplan in a target list, it is executed after the join in my case.
> The subplan
> can be execute after the scan of T1(see below example) and it can also be
> executed
> after the join. Which one is better depends on which methods make the
> num_exec
> smaller. Is it something we already considered?
Uh, I'm not following your concern. SubPlans appearing in the join
targetlist *must* be executed "after the join", ie only for valid
join rows. Otherwise we could have cases where, say, they throw
errors that should not occur.
I am feeling I'm wrong somewhere however I can't figure it out until now.
Q1: select (select t3.a from t3 where t3.c = t1.c) from t1, t2 where t1.b = t2.b;
should equals Q2:
1. select (select t3.a from t3 where t3.c = t1.c) as a, b from t1 ==> t13.
2. select t13.a from t13, t2 where t13.b = t2.b;
With the following data, Q1 will execute the subplan twice (since we get 2 rows
after join t1, t2). while Q2 executes the subplan once (since t1 has only 1 row).
Finally the result is the same.
postgres=# select * from t1;
a | b | c
---+---+---
1 | 1 | 1
(1 row)
postgres=# select * from t2;
a | b | c
---+---+---
1 | 1 | 1
1 | 1 | 2
(2 rows)
postgres=# select * from t3;
a | b | c
---+---+---
1 | 1 | 1
(1 row)
a | b | c
---+---+---
1 | 1 | 1
(1 row)
postgres=# select * from t2;
a | b | c
---+---+---
1 | 1 | 1
1 | 1 | 2
(2 rows)
postgres=# select * from t3;
a | b | c
---+---+---
1 | 1 | 1
(1 row)
On the other hand, SubPlans appearing
in the join's qual conditions have to be executed "before the join",
although exactly what that means is fuzzy because we don't make any
promises about the relative ordering of different qual conditions.
> When the subplan is in a Qual, it is supposed to be executed as soon as
> possible,
> The current implementation matches the below cases. So can we say we
> knows the num_execs of SubPlan just after we plan the dependent rels?
I wouldn't say so. If the SubPlan's qual actually only depends on one
of the input rels, distribute_qual_to_rels would have pushed it down
further than the join. Among the quals that do have to be evaluated at the join,
a qual involving a SubPlan is best executed last on cost
grounds, or so I'd guess anyway. So the number of executions is probably
less than the product of the input rel sizes. That's what motivates
the choice of NUM_EXEC_QUAL in my patch.
Best Regards
Andy Fan
On Tue, 1 Sep 2020 at 05:22, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: > > One inefficiency I see that we could probably get rid of is > > where make_subplan() is doing > > /* Now we can check if it'll fit in hash_mem */ > > /* XXX can we check this at the Path stage? */ > > I went ahead and fixed that, and I also realized there's another small > improvement to be made: we can remove the unused SubPlan from the > subplans list of the finished PlannedStmt, by setting that list cell > to NULL. (This is already specifically allowed by the comments for > PlannedStmt.subplans.) Initially I supposed that this'd only save the > costs of copying that subtree when we copy the whole plan. On looking > closer, though, InitPlan actually runs ExecInitNode on every list > entry, even unused ones, so this will make some difference in executor > startup time. > > Hence, an updated 0001 patch. 0002 hasn't changed. I had a look over these two. A handful of very small things: 0001: 1. I think we should be moving away from using linitial() and second() when we know there are two items in the list. Using list_nth() has less overhead. subplan1 = (SubPlan *) linitial(asplan->subplans); subplan2 = (SubPlan *) lsecond(asplan->subplans); 2. I did have sight concerns that fix_alternative_subplan() always assumes the list of subplans will always be 2, though on looking at the definition of AlternativeSubPlan, I see always having two in the list is mentioned. It feels like fix_alternative_subplan() wouldn't become much more complex to allow any non-zero number of subplans, but maybe making that happen should wait until there is some need for more than two. It just feels a bit icky to have to document the special case when not having the special case is not that hard to implement. 3. Wouldn't it be better to say NULLify rather than delete? + * node or higher-level nodes. However, we do delete the rejected subplan + * from root->glob->subplans, to minimize cycles expended on it later. 0002: I don't have much to say about this. Leaving the code in get_rule_expr() for the reasons you mentioned in the new comment does make sense. On a side note, I was playing around with the following case: create table t (a int, b int, c int); insert into t select x,1,2 from generate_Series(1,10000)x; create index on t (b); vacuum freeze analyze t; and ran: select * from t where exists (select 1 from t t2 where t.a=t2.b) or a < 0; EXPLAIN ANALYZE shows: QUERY PLAN ---------------------------------------------------------------------------------------------------------------- Seq Scan on t (cost=0.00..360.00 rows=5000 width=12) (actual time=0.020..7468.452 rows=1 loops=1) Filter: ((SubPlan 1) OR (a < 0)) Rows Removed by Filter: 9999 SubPlan 1 -> Seq Scan on t t2 (cost=0.00..180.00 rows=10000 width=0) (actual time=0.746..0.746 rows=0 loops=10000) Filter: (t.a = b) Rows Removed by Filter: 9999 Planning Time: 0.552 ms Execution Time: 7468.481 ms (9 rows) Notice that the SubPlan's estimated rows are 10000. This is due to the ndistinct for "b" being 1 and since t.a is a parameter, the selectivity is estimated to be 1.0 by var_eq_non_const(). Unfortunately, for this reason, the index on t(b) is not used either. The planner thinks all rows are being selected, in which case, an index is not much help. both master and patched seem to not choose to use the hashed subplan which results in a pretty slow execution time. This seems to be down to cost_subplan() doing: /* we only need to fetch 1 tuple; clamp to avoid zero divide */ sp_cost.per_tuple += plan_run_cost / clamp_row_est(plan->plan_rows); I imagine / 2 might be more realistic to account for the early abort, which is pretty much what the ALL_SUBLINK and ANY_SUBLINK do just below: Changing that makes the run-time of that query go from 7.4 seconds for me down to 3.7 ms, about 2000 times faster. I understand there will be other cases where that's not so ideal, but this slowness is not ideal either. Of course, not the fault of this patch. David
Thanks for reviewing! David Rowley <dgrowleyml@gmail.com> writes: > 1. I think we should be moving away from using linitial() and second() > when we know there are two items in the list. Using list_nth() has > less overhead. Uh, really? And if it's true, why would we change all the call sites rather than improving the pg_list.h macros? > 2. I did have sight concerns that fix_alternative_subplan() always > assumes the list of subplans will always be 2, though on looking at > the definition of AlternativeSubPlan, I see always having two in the > list is mentioned. It feels like fix_alternative_subplan() wouldn't > become much more complex to allow any non-zero number of subplans, but > maybe making that happen should wait until there is some need for more > than two. It just feels a bit icky to have to document the special > case when not having the special case is not that hard to implement. It seemed to me that dealing with the general case would make fix_alternative_subplan() noticeably more complex and less obviously correct. I might be wrong though; what specific coding did you have in mind? > 3. Wouldn't it be better to say NULLify rather than delete? > + * node or higher-level nodes. However, we do delete the rejected subplan > + * from root->glob->subplans, to minimize cycles expended on it later. Fair enough, that comment could be improved. > On a side note, I was playing around with the following case: > ... > both master and patched seem to not choose to use the hashed subplan > which results in a pretty slow execution time. This seems to be down > to cost_subplan() doing: > /* we only need to fetch 1 tuple; clamp to avoid zero divide */ > sp_cost.per_tuple += plan_run_cost / clamp_row_est(plan->plan_rows); > I imagine / 2 might be more realistic to account for the early abort, > which is pretty much what the ALL_SUBLINK and ANY_SUBLINK do just > below: Hm, actually isn't it the other way around? *If* there are any matching rows, then what's being done here is an accurate estimate. But if there are not, we're going to have to scan the entire subquery output to verify that. I wonder if we should just be taking the subquery cost at face value, ie be pessimistic not optimistic. If the user is bothering to test EXISTS, we should expect that the no-match case does happen. However, I think that's a distinct concern from this patch; this patch is only meant to improve the processing of alternative subplans, not to change the costing rules around them. If we fool with it I'd rather do so as a separate patch. regards, tom lane
On Sun, 27 Sep 2020 at 10:03, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Thanks for reviewing! > > David Rowley <dgrowleyml@gmail.com> writes: > > 1. I think we should be moving away from using linitial() and second() > > when we know there are two items in the list. Using list_nth() has > > less overhead. > > Uh, really? Yeah. Using linitial() and lsecond() will check if the list is not-NIL. lsecond() does an additional check to ensure the list has at least two elements. None of which are required since we already know the list has two elements. > And if it's true, why would we change all the call sites > rather than improving the pg_list.h macros? Maybe we should. Despite the non-NIL check and length check in list_head(), list_second_cell(), list_third_cell() functions, the corresponding macro will crash anyway if those functions were to return NULL. We might as well just use list_nth_cell() to get the ListCell without any checks to see if the cell exists. I can go off and fix those separately. I attached a 0004 patch to help explain what I'm talking about. > > 2. I did have sight concerns that fix_alternative_subplan() always > > assumes the list of subplans will always be 2, though on looking at > > the definition of AlternativeSubPlan, I see always having two in the > > list is mentioned. It feels like fix_alternative_subplan() wouldn't > > become much more complex to allow any non-zero number of subplans, but > > maybe making that happen should wait until there is some need for more > > than two. It just feels a bit icky to have to document the special > > case when not having the special case is not that hard to implement. > > It seemed to me that dealing with the general case would make > fix_alternative_subplan() noticeably more complex and less obviously > correct. I might be wrong though; what specific coding did you have in > mind? I had thought something like 0003 (attached). It's a net reduction of 3 entire lines, including the removal of the comment that explained that there's always two in the list. > > On a side note, I was playing around with the following case: > > ... > > both master and patched seem to not choose to use the hashed subplan > > which results in a pretty slow execution time. This seems to be down > > to cost_subplan() doing: > > /* we only need to fetch 1 tuple; clamp to avoid zero divide */ > > sp_cost.per_tuple += plan_run_cost / clamp_row_est(plan->plan_rows); > > I imagine / 2 might be more realistic to account for the early abort, > > which is pretty much what the ALL_SUBLINK and ANY_SUBLINK do just > > below: > > Hm, actually isn't it the other way around? *If* there are any matching > rows, then what's being done here is an accurate estimate. But if there > are not, we're going to have to scan the entire subquery output to verify > that. I wonder if we should just be taking the subquery cost at face > value, ie be pessimistic not optimistic. If the user is bothering to > test EXISTS, we should expect that the no-match case does happen. > > However, I think that's a distinct concern from this patch; this patch > is only meant to improve the processing of alternative subplans, not > to change the costing rules around them. If we fool with it I'd rather > do so as a separate patch. Yeah, agreed. I'll open another thread. David
Attachment
David Rowley <dgrowleyml@gmail.com> writes: > On Sun, 27 Sep 2020 at 10:03, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> And if it's true, why would we change all the call sites >> rather than improving the pg_list.h macros? > Maybe we should. Despite the non-NIL check and length check in > list_head(), list_second_cell(), list_third_cell() functions, the > corresponding macro will crash anyway if those functions were to > return NULL. Hm, good point. > We might as well just use list_nth_cell() to get the > ListCell without any checks to see if the cell exists. I can go off > and fix those separately. I attached a 0004 patch to help explain what > I'm talking about. Yeah, that should be dealt with separately. >> It seemed to me that dealing with the general case would make >> fix_alternative_subplan() noticeably more complex and less obviously >> correct. I might be wrong though; what specific coding did you have in >> mind? > I had thought something like 0003 (attached). It's a net reduction of > 3 entire lines, including the removal of the comment that explained > that there's always two in the list. Meh. This seems to prove my point, as it's in fact wrong; you are only nulling out the discarded subplans-list entry in one of the two cases. Once you fix that it's not really shorter anymore, nor clearer. Still, I suppose there's some value in removing the assumption about exactly two subplans. I'll fix up fix_alternative_subplan and push this. I think the other topics should be raised in separate threads. Thanks for reviewing! regards, tom lane