Respect disabled_nodes in fix_alternative_subplan.

When my commit e22253467942fdb100087787c3e1e3a8620c54b2 added the
concept of disabled_nodes, it failed to add a disabled_nodes field
to SubPlan. This is a regression: before that commit, when
fix_alternative_subplan compared the costs of two plans, the number
of disabled nodes affected the result, because it was just a
component of the total cost. After that commit, it no longer did,
making it possible for a disabled path to win on cost over one that
is not disabled. Fix that.

As usual for planner fixes that might destabilize plan choices,
no back-patch.

Discussion: https://postgr.es/m/CA+TgmoaK=4w7-qknUo3QhUJ53pXZq=c=KgZmRyD+k7ytqfmgSg@mail.gmail.com
Reviewed-by: Lukas Fittl <lukas@fittl.com>

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 56d45287c8962faf4f019682e4c573a2b06d6d5a..1c575e56ff6074ee46d41368ab58beedee39384a 100644 (file)
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -4761,6 +4761,7 @@ cost_subplan(PlannerInfo *root, SubPlan *subplan, Plan *plan)
                        sp_cost.per_tuple += plan->startup_cost;
        }
 
+       subplan->disabled_nodes = plan->disabled_nodes;
        subplan->startup_cost = sp_cost.startup;
        subplan->per_call_cost = sp_cost.per_tuple;
 }

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1b5b9b5ed9cc8cd5214cab7d1f7d531fc1cb90ad..ff0e875f2a2278fcd604795d9adba93a77858405 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2234,9 +2234,12 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
 
        /*
         * Compute the estimated cost of each subplan assuming num_exec
-        * executions, and keep the cheapest one.  In event of exact equality of
-        * estimates, we prefer the later plan; this is a bit arbitrary, but in
-        * current usage it biases us to break ties against fast-start subplans.
+        * executions, and keep the cheapest one.  If one subplan has more
+        * disabled nodes than another, choose the one with fewer disabled nodes
+        * regardless of cost; this parallels compare_path_costs.  In event of
+        * exact equality of estimates, we prefer the later plan; this is a bit
+        * arbitrary, but in current usage it biases us to break ties against
+        * fast-start subplans.
         */
        Assert(asplan->subplans != NIL);
 
@@ -2246,7 +2249,10 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
                Cost            curcost;
 
                curcost = curplan->startup_cost + num_exec * curplan->per_call_cost;
-               if (bestplan == NULL || curcost <= bestcost)
+               if (bestplan == NULL ||
+                       curplan->disabled_nodes < bestplan->disabled_nodes ||
+                       (curplan->disabled_nodes == bestplan->disabled_nodes &&
+                        curcost <= bestcost))
                {
                        bestplan = curplan;
                        bestcost = curcost;

diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b67e56e6c5abdf969ec9a9488e20e9183b247bcb..f5b6b45664ae0a29d7b2825d83e2b56532565abf 100644 (file)
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1124,6 +1124,7 @@ typedef struct SubPlan
        List       *parParam;           /* indices of input Params from parent plan */
        List       *args;                       /* exprs to pass as parParam values */
        /* Estimated execution costs: */
+       int                     disabled_nodes; /* count of disabled nodes in the plan */
        Cost            startup_cost;   /* one-time setup cost */
        Cost            per_call_cost;  /* cost for each subplan evaluation */
 } SubPlan;