On Thu, Jan 29, 2026 at 9:10 AM Robert Haas <[email protected]> wrote:
> I think cost_material() got an enabled argument because of
> materialize_finished_plan(). Most of the time, plan nodes are created
> from paths, and we want to use the parent's pgs_mask to determine
> whether the chosen strategy (e.g. materialization) is enabled.
> However, materialize_finished_plan() creates a plan directly, so
> there's no RelOptInfo or JoinPathExtraData whose pgs_mask we can
> consult, and so we have to fall back on consulting enable_material
> directly. Once that parameter got added to cost_material(), it made
> sense to me to add it to create_material_path() as well.

If this explanation seems a little weak, it's because it was. Here's a
better explanation: the patch added a per-rel flag
PGS_NESTLOOP_MATERIALIZE that enables the use of a Nested Loop join
with an inner Materialize node. When we construct such a path, the
"parent" point for the materialize path points to the rel that is on
the inner side of the join, not the joinrel itself. But the pgs_mask
flags of the joinrel and its inner side could be different. Therefore,
the Materialize node can't just look at path->parent->pgs_mask to
decide whether to mark the node as disabled. So, when I wrote the
patch originally, I added an enabled flag here to make sure that we
pass down the information about whether the join method was enabled at
the joinrel level, since the joinrel's pgs_mask is not otherwise
available to cost_material(). Later on, I discovered the need for
PGS_CONSIDER_NONPARTIAL, but by then I had forgotten why that
"enabled" argument existed and pushed the logic to handle
PGS_CONSIDER_NONPARTIAL into cost_material(). So the logic as
currently committed is buggy in the case where the joinrel and the
innerrel have differing pgs_mask values. match_unsorted_outer() and
consider_parallel_nestloop() have the intention of generating
Materialize nodes only when materialization is enabled, so that the
Materialize nodes are never marked disabled. But with the patch as
committed, a non-partial Nested Loop with inner Materialize can
disable the Materialize node if the inner rel's pgs_mask has
PGS_CONSIDER_NONPARTIAL unset.

The right fix is to make all the decisions about whether the
Materialize nod should be created, and whether it should be enabled,
in the caller, and have none of that logic in cost_material(). I will
post a new patch set shortly, which will include a patch to rectify
this issue.

-- 
Robert Haas
EDB: http://www.enterprisedb.com


Reply via email to