From cbdf93d4712229fd82d40d823882a5bc84e407e5 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 10 Feb 2026 11:49:07 -0500 Subject: [PATCH] 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 --- src/backend/optimizer/path/costsize.c | 5 ----- src/backend/optimizer/path/joinpath.c | 11 ++++++++++- src/backend/optimizer/plan/createplan.c | 4 ---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index c30d6e84672..89ca4e08bf1 100644 --- 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 1e4246b49d5..e0c00e26dd5 100644 --- 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 e5200f4b3ce..a50260290fa 100644 --- 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, -- 2.47.3