Horton
Menu

Re: pg_plan_advice

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Lukas Fittl <lukas@fittl.com>, Tom Lane <tgl@sss.pgh.pa.us>, Jacob Champion <jacob.champion@enterprisedb.com>, Dian Fay <di@nmfay.com>, Matheus Alcantara <matheusssilv97@gmail.com>, Jakub Wartak <jakub.wartak@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-01-29T14:10:11Z
Lists: pgsql-hackers 
On Thu, Jan 29, 2026 at 1:44 AM Richard Guo <guofenglinux@gmail.com> wrote:
> I think we should access spath->disabled_nodes until after we have
> verified that spath is not NULL.

Good catch. I have included a fix for this in
4020b370f214315b8c10430301898ac21658143f.

> Also, it's not quite clear to me why create_material_path (and
> cost_material) requires the "enabled" parameter.  I couldn't find any
> comments on this, so it might be worth adding some comments here.

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.

There are probably other ways I could have done this. For instance, we
could make create_material_path() pass true to cost_material(), and
not have its own "enabled" argument. That would mean that
reparameterize_path() would have to unconditionally pass true. I'm not
sure if that would be a problem. Another idea is to make
cost_material() consult enable_material itself if path->parent is
NULL, so that it wholly encapsulates the calculation locally.

One thing that is a little different about materialization vs. some
other operations is that it's used for multiple purposes. Sometimes we
use it because it's required for correctness, and other times we do it
to reduce cost. And, it can be used to affect cost in different
scenarios. In the Nested Loop case, PGS_NESTLOOP_MATERIALIZE should
control whether we are willing to materialize, but that doesn't apply
to other cases, like build_subplan() or create_mergejoin_plan().
Moreover, these latter cases have not yet been converted to use Paths
-- hopefully they will be converted someday, but maybe not soon. All
of this makes things a little more complicated for materialization vs.
something like a nested loop or a sequential scan, where the logic to
create that thing is all in one place and paths are in use.

But that is not to say that I'm deeply invested in cost_material()
having an "enabled" argument -- that seems like a detail to me that
could go either way. If it seems objectionable, we could change it,
and I think we could probably do so in more than one way and without
really affecting the behavior. Alternatively, we could leave it the
way it is and add a comment clarifying whatever portion of the above
you think is worth documenting in the source. It seemed like a
sufficiently minor detail to me that I did not bother, but that might
have been a mistake.

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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. pg_plan_advice: Fix another unique-semijoin bug.

  2. pg_plan_advice: Export feedback-related definitions.

  3. pg_plan_advice: Fix a bug when a subquery is pruned away entirely.

  4. pg_plan_advice: Add alternatives test to Makefile.

  5. pg_plan_advice: Handle non-repeatable TABLESAMPLE scans.

  6. pg_stash_advice: Allow stashed advice to be persisted to disk.

  7. Add pg_stash_advice contrib module.

  8. pg_plan_advice: Avoid assertion failure with partitionwise aggregate.

  9. pg_plan_advice: Invent DO_NOT_SCAN(relation_identifier).

  10. Add an alternative_plan_name field to PlannerInfo.

  11. pg_plan_advice: Refactor to invent pgpa_planner_info

  12. Respect disabled_nodes in fix_alternative_subplan.

  13. get_memoize_path: Don't exit quickly when PGS_NESTLOOP_PLAIN is unset.

  14. test_plan_advice: Set TAP test priority 50 in meson.build.

  15. pg_plan_advice: Avoid a crash under GEQO.

  16. Test pg_plan_advice using a new test_plan_advice module.

  17. pg_plan_advice: Always install pg_plan_advice.h, and in the right place

  18. pg_plan_advice: Fix failures to accept identifier keywords.

  19. Add pg_plan_advice contrib module.

  20. Allow extensions to mark an individual index as disabled.

  21. Replace get_relation_info_hook with build_simple_rel_hook.

  22. Store information about Append node consolidation in the final plan.

  23. Store information about elided nodes in the final plan.

  24. Store information about range-table flattening in the final plan.

  25. Pass cursorOptions to planner_setup_hook.

  26. Fix PGS_CONSIDER_NONPARTIAL interaction with Materialize nodes.

  27. Fix mistakes in commit 4020b370f214315b8c10430301898ac21658143f

  28. Allow for plugin control over path generation strategies.

  29. Update some comments for fasthash

  30. Allow passing a pointer to GetNamedDSMSegment()'s init callback.

  31. Don't reset the pathlist of partitioned joinrels.

  32. Treat number of disabled nodes in a path as a separate cost metric.