Re: pg_plan_advice

On Sat, Mar 21, 2026 at 9:13 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> So I'm left with the idea that to get test_plan_advice to be fully
> stable on these slower machines, it will probably be necessary to make
> it control which AlternativeSubPlan is chosen and whether a
> MinMaxAggPath is chosen or not. I have some ideas about how to
> accomplish that in a reasonably elegant fashion without adding too
> much new machinery, but I need to spend some more time validating
> those ideas before committing to a precise course of action. More
> soon.

Here is v22. There are four new patches.

0001 adds a disabled_nodes fields to SubPlan, to fix the bug that I
identified in the email to which this is a reply.

0002-0004 are an attempt to fix the remaining buildfarm failures not
already addressed (or attempted to be addressed, anyway) by other
commits. The basic idea, implemented by 0004, is to add a
DO_NOT_SCAN() advice tag. This advice is generated when we consider a
MinMaxAggPath or a hashed SubPlan. In either case, all relations which
are part of the non-selected alternative are marked DO_NOT_SCAN(),
which works like scan type advice but disables every possible scan
type rather than still allowing exactly one of them. Unless I've
missed something, this should be sufficient to make pg_plan_advice
stabilize which of two alternative SubPlans we pick and whether or not
a min/max aggregate is chosen. 0002 does some preliminary refactoring
to provide a more centralized way of tracking per-PlannerInfo details
within pg_plan_advice. 0003 makes the necessary change to
src/backend/optimizer, which consists of adding an alternative_root
field to each PlannerInfo and setting it appropriately. 0004 then
updates pg_plan_advice to implement DO_NOT_SCAN().

0005 is the pg_collect_advice module from previous versions of the
patch set. The main change here is that I completely rewrote the TAP
test, which previously was running the entire regression test suite
yet another time. That's been replaced with something that is much
faster and much better targeted at properly testing the shared advice
collector. Aside from that, I added one more check for
InvalidDsaPointer where the code was previously lacking one.

0006 is the pg_stash_advice module from previous versions of the patch
set. I have adjusted this to be much safer against permanent DSA
leaks. It now uses dshash_find_or_insert_extended instead of relying
on the ability to dshash_find a just-inserted entry without error. It
now also holds an LWLock while inserting or updating an entry in the
dshash table, for reasons explained in the comments. On the other
hand, it no longer unnecessarily holds the LWLock in exclusive mode
when looking up advice strings for automatic application, which was a
rather silly mistake in the previous version. A few additional tests
have been added. Alphabetization in contrib/Makefile has been fixed.

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