Horton
Menu

Re: pg_plan_advice

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Alexandra Wang <alexandra.wang.oss@gmail.com>, Richard Guo <guofenglinux@gmail.com>, 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-03-04T15:17:37Z
Lists: pgsql-hackers 
On Fri, Feb 27, 2026 at 8:16 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
> I have a mind to walk through the readmes and sgmls but its going to be in chunks.  Here's one for the readme for pg_plan_advice with a couple of preliminary sgml changes.

While I'm grateful for the feedback, I feel like you tend to suggest a
lot of edits that seem like they're just substituting your
idiosyncratic preferences for mine e.g. writing "types of scan" vs.
"scan types," or writing "additional, separate join problems" vs.
"independent join problems" or "judiciously" vs. "conservatively". I
don't really consider these to be improvements, nor do I necessarily
think they're worse, but I just don't see the point in litigating this
kind of stuff. If I've written something that is legitimately unclear,
or factually incorrect, or there's a spelling or punctuation mistake,
I'm happy to correct that kind of stuff, but I don't really want to go
through and replace a bunch of words that I liked with a bunch of
synonyms that you picked.

Also, when you just provide a diff like this, it's not that clear to
me why you're suggesting particular changes, which makes it hard to
decide whether I agree with them. And in a lot of cases I don't.
Looking at some particular examples:

+isn't going to work any more. That's expected. It should be resilient to
+changes in the statistics, including any CREATE STATISTICS related changes.

This is broadly true but seems a bit obvious to mention in a README.
If we were going to mention it, I'd think it would go in the
user-facing documentation. But I don't quite see why we should mention
it at all. If plan advice couldn't override changes caused by
statistics, what would even be the point of it? Also, it's not
categorically true in all situations, because as discussed elsewhere,
we have limitations like lack of control over aggregation strategy.

 Tags such as NESTED_LOOP_PLAIN specify the method that should be used to
-perform a certain join. More specifically, NESTED_LOOP_PLAIN(x (y z)) says
+perform a certain join - with the target appearing directly on the inner side
+of the join list first. Thus, NESTED_LOOP_PLAIN(x (y z)) says
 that the plan should put the relation whose identifier is "x" on the inner
 side of a plain nested loop (one without materialization or memoization)
 and that it should also put a join between the relation whose identifier is

This seems like you're adding a second explanation of what the
paragraph already goes onto say, except that the existing explanation
is more precise and detailed.

-useless in practice. It gives the planner too much freedom to do things that
+problematic in practice. It gives the planner too much freedom to do
things that

I mean, I stand by the word I picked. I don't want to weaken it.

-This means that if advice can say that a certain optimization or technique
-should be used, it should also be able to say that the optimization or
-technique should not be used. We should never assume that the absence of an
-instruction to do a certain thing means that it should not be done; all
-instructions must be explicit.
+In other words, advice tags must define whether they encourage or discourage
+certain optimizations or techniques. (NO_GATHER is an example of the latter.
+There is no generic "NOT" syntax, e.g., NOT(HASH_JOIN(dim2 dim4).))

My text explains an important design principle that future hackers
must keep in mind when modifying this system to avoid breaking
everything. Your replacement text just describes how it works today.
Considering that this is a README for hackers, I think that's much
worse.

-  advice" mini-language. It is intended to allow stabilization of plan choices
+  advice" domain specific language (DSL). It is intended to allow
stabilization of plan choices

There's a debate to be had about whether it's better to say
mini-language or domain specific language here, but it's hard for me
to decide which is better if all you provide is a diff replacing A
with B. I definitely think it's worse to write (DSL) here. There is no
point in defining an acronym if we're never going to use it anywhere.

-- 
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.