On Thu, Mar 19, 2026 at 12:46 PM Tom Lane <[email protected]> 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


Reply via email to