Fix add_partial_path interaction with disabled_nodes

Commit e22253467942fdb100087787c3e1e3a8620c54b2 adjusted the logic in
add_path() to keep the path list sorted by disabled_nodes and then by
total_cost, but failed to make the corresponding adjustment to
add_partial_path. As a result, add_partial_path might sort the path list
just by total cost, which could lead to later planner misbehavior.

In principle, this should be back-patched to v18, but we are typically
reluctant to back-patch planner fixes for fear of destabilizing working
installations, and it is unclear to me that this has sufficiently
serious consequences to justify an exception, so for now, no back-patch.

Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: http://postgr.es/m/CAMbWs4-mO3jMK4t_LgcJ+7Eo=NmGgkxettgRaVbJzZvVZ1koMA@mail.gmail.com

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 9678c20ff1f17ea0088f79ceef25e35053213b59..ef8ef6e89d377fd0619959bd3294afcee9c84dc2 100644 (file)
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -758,9 +758,10 @@ add_path_precheck(RelOptInfo *parent_rel, int disabled_nodes,
  *       parallel such that each worker will generate a subset of the path's
  *       overall result.
  *
- *       As in add_path, the partial_pathlist is kept sorted with the cheapest
- *       total path in front.  This is depended on by multiple places, which
- *       just take the front entry as the cheapest path without searching.
+ *       As in add_path, the partial_pathlist is kept sorted first by smallest
+ *    number of disabled nodes and then by lowest total cost. This is depended
+ *    on by multiple places, which just take the front entry as the cheapest
+ *    path without searching.
  *
  *       We don't generate parameterized partial paths for several reasons.  Most
  *       importantly, they're not safe to execute, because there's nothing to
@@ -878,8 +879,13 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
                }
                else
                {
-                       /* new belongs after this old path if it has cost >= old's */
-                       if (new_path->total_cost >= old_path->total_cost)
+                       /*
+                        * new belongs after this old path if it has more disabled nodes
+                        * or if it has the same number of nodes but a greater total cost
+                        */
+                       if (new_path->disabled_nodes > old_path->disabled_nodes ||
+                               (new_path->disabled_nodes == old_path->disabled_nodes &&
+                                new_path->total_cost >= old_path->total_cost))
                                insert_at = foreach_current_index(p1) + 1;
                }