Fix PGS_CONSIDER_NONPARTIAL interaction with Materialize nodes.

Commit 4020b370f214315b8c10430301898ac21658143f had the idea that it
would be a good idea to handle testing PGS_CONSIDER_NONPARTIAL within
cost_material to save callers the trouble, but that turns out not to be
a very good idea. One concern is that it makes cost_material() dependent
on the caller having initialized certain fields in the MaterialPath,
which is a bit awkward for materialize_finished_plan, which wants to use
a dummy path.

Another problem is that it can result in generated materialized nested
loops where the Materialize node is disabled, contrary to the intention
of joinpath.c's logic in match_unsorted_outer() and
consider_parallel_nestloop(), which aims to consider such paths only
when they would not need to be disabled. In the previous coding, it was
possible for the pgs_mask on the joinrel to have PGS_CONSIDER_NONPARTIAL
set, while the inner rel had the same bit clear. In that case, we'd
generate and then disable a Materialize path.

That seems wrong, so instead, pull up the logic to test the
PGS_CONSIDER_NONPARTIAL bit into joinpath.c, restoring the historical
behavior that either we don't generate a given materialized nested loop
in the first place, or we don't disable it.

Discussion: http://postgr.es/m/CA+TgmoawzvCoZAwFS85tE5+c8vBkqgcS8ZstQ_ohjXQ9wGT9sw@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoYS4ZCVAF2jTce=bMP0Oq_db_srocR4cZyO0OBp9oUoGg@mail.gmail.com

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index c30d6e84672f0d27eae1923983ea3af0d46191df..89ca4e08bf156044bde234b1a56893e4de3150c2 100644 (file)
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2589,11 +2589,6 @@ cost_material(Path *path,
        double          nbytes = relation_byte_size(tuples, width);
        double          work_mem_bytes = work_mem * (Size) 1024;
 
-       if (path->parallel_workers == 0 &&
-               path->parent != NULL &&
-               (path->parent->pgs_mask & PGS_CONSIDER_NONPARTIAL) == 0)
-               enabled = false;
-
        path->rows = tuples;
 
        /*

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 1e4246b49d5bea8b3f593a43e1745b26c274a0c2..e0c00e26dd5d57ffd90c8846a61f32f3b372cae3 100644 (file)
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -1895,8 +1895,17 @@ match_unsorted_outer(PlannerInfo *root,
                /*
                 * Consider materializing the cheapest inner path, unless that is
                 * disabled or the path in question materializes its output anyway.
+                *
+                * At present, we only consider materialization for non-partial outer
+                * paths, so it's correct to test PGS_CONSIDER_NONPARTIAL here. If we
+                * ever want to consider materialization for partial paths, we'll need
+                * to create matpath whenever PGS_NESTLOOP_MATERIALIZE is set, use it
+                * for partial paths either way, and use it for non-partial paths only
+                * when PGS_CONSIDER_NONPARTIAL is also set.
                 */
-               if ((extra->pgs_mask & PGS_NESTLOOP_MATERIALIZE) != 0 &&
+               if ((extra->pgs_mask &
+                        (PGS_NESTLOOP_MATERIALIZE | PGS_CONSIDER_NONPARTIAL)) ==
+                       (PGS_NESTLOOP_MATERIALIZE | PGS_CONSIDER_NONPARTIAL) &&
                        inner_cheapest_total != NULL &&
                        !ExecMaterializesOutput(inner_cheapest_total->pathtype))
                        matpath = (Path *)

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index e5200f4b3ced3e39f082eeb80de6cc83002346fa..a50260290fac98853a213967ae8d3f896e56e7e6 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -6526,10 +6526,6 @@ materialize_finished_plan(Plan *subplan)
        subplan->startup_cost -= initplan_cost;
        subplan->total_cost -= initplan_cost;
 
-       /* Clear fields that cost_material() will consult */
-       matpath.parallel_workers = 0;
-       matpath.parent = NULL;
-
        /* Set cost data */
        cost_material(&matpath,
                                  enable_material,