Horton
Menu

Re: pg_plan_advice (now with transparent SQL plan performance overrides - pg_stash_advice)

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>, 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>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-03-04T13:37:33Z
Lists: pgsql-hackers 
On Tue, Mar 3, 2026 at 11:11 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
> Ok, that’s what I was missing here, it saw the subplan in its options but the winning plan didn’t include it.  So “matched/failed” may produce a plan where the target having been matched isn’t actually visible to the user.
>
> Maybe add a note like this to pg_plan_advice:
>
> Generated advice is produced to a high level of specificity without knowing what limitations the advice interpreter has in applying that advice.  Therefore, generated advice targets may later fail for no other reason than cost-based decisions resulted in the originally chosen plan to no longer be chosen.  The planner will still likely see the original target on a now losing plan and thus the advice feedback will report matched even when the winning plan does not include the specific target.

There is doubtless lots of room for improvement in the documentation,
but this specific text doesn't seem like a good idea, because it's not
true in general. The whole point of test_plan_advice is to ensure that
generated advice targets *don't* fail, and with all the patches
applied, I get a clean run where every single advice target generated
by the main regression test suite can be successfully applied back to
the plan that generated it. I think the real issue here is that Jakub
has found a case where fiddling with the scan method causes the
optimal aggregation method to change, and the resulting weird behavior
stems from the fact that aggregation control is not supported. What
you want to be able to do is nail down the aggregation behavior first,
and then it would be clear whether to provide scan advice for t1 or
for t1@minmax_1.

But I just don't see what the big deal is here. One thing you can do
is provide advice for both t1 and t1@minmax_1 and then the advice will
be followed for whichever one appears in the final plan. The other
will be marked as failed, but you can decide to just ignore that.
Possibly you could even indirectly control whether the minmax path is
selected by advising an inefficient strategy for the one you don't
want to be be picked and a great strategy for the one you do want to
be picked. Also, I've personally never run into a real-world case
where the planner made a bad decision about whether to use a minmaxagg
or a regular agg, which is why all of my development time and effort
went into scans and joins and related topics where I do regularly see
things go wrong.

So at the risk of repeating myself: It is not in general true that you
should expect your advice to randomly not work, but that happens in
this case because aggregation control is not supported, which is a
documented limitation.

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