Horton
Menu

Re: pg_plan_advice

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-03-19T17:17:04Z
Lists: pgsql-hackers 
On Thu, Mar 19, 2026 at 12:46 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I just realized that this new test module has added yet another
> execution of the entire core regression test suite ... and to add
> insult to injury, it runs the planner twice for each plannable
> query therein.  I think this is an outrageous abuse of our buildfarm
> and urgently request that you find some less climate-destroying
> way of getting reasonable test coverage of pg_plan_advice.

I don't think just nuking this is a reasonable option. During the
development of pg_plan_advice, test_plan_advice was the single most
valuable testing tool. It was not close. Manual review, both by me and
by others, found bugs; fuzz testing by Jacob Champion found bugs; AI
code review found bugs. test_plan_advice was an order of magnitude
more effective than all of those things put together. In fact, I'd say
maybe two orders of magnitude. Pretty much every significant logical
error was something that required a very specific query shape to find,
and the regression tests are by far the best repository of weird query
shapes that we have.

I think without something like test_plan_advice, the chances of us
noticing if future planner changes break pg_plan_advice are near zero,
and with it, they're quite good -- because we're not only testing the
queries that exist in the regression test suite now, but we're testing
future queries that are added to the regression test suite by people
who are hacking on the planner. Those added queries presumably create
plan shapes that are relevant to the code that they're changing,
whereas a fixed pg_plan_advice-only works if people think to add
something to it, which they very likely won't, and even if they do, I
have no confidence that they'll know what things to add and what not.
I certainly didn't know which queries in the regression tests were
going to break under test_plan_advice until I tried it.

One thing we might be able to do here to save on cycles is combine
test_plan_advice with some existing run of the regression tests. We
seem to run them to test sepgsql, to test pg_upgrade, and in
027_stream_regress.pl. I don't think we can piggyback on sepgsql here
because there are too many cases where it just won't be built or
tested, but we could possibly piggyback on one of the other runs, or
on the main regression tests. If that's not viable, another option
would be to have it run on only some buildfarm members rather than all
of them.  But if the tests aren't run by default, then I fear people
will experience quite a bit frustration the first time something
breaks only after they commit.

Before I forget, another idea that might help is to see if we can
tweak meson.build to start running this particular test earlier. I
thought about that during development, but I didn't actually do it. If
the issue is that test being last to finish, that could help. If the
issue is total resource consumption, it won't help with that.

> I would dig into why grison and schnauzer are failing this test,
> except that I don't agree that we should be running it in the
> first place.

I'll go have a look.

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