Horton

Re: pg_plan_advice

Jacob Champion <jacob.champion@enterprisedb.com>

View thread
From: Jacob Champion <jacob.champion@enterprisedb.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dian Fay <di@nmfay.com>, Matheus Alcantara <matheusssilv97@gmail.com>, Jakub Wartak <jakub.wartak@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-12-09T01:18:50Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add pg_plan_advice contrib module.

  2. Store information about Append node consolidation in the final plan.

  3. Store information about elided nodes in the final plan.

  4. Store information about range-table flattening in the final plan.

  5. Allow for plugin control over path generation strategies.

  6. Allow passing a pointer to GetNamedDSMSegment()'s init callback.

  7. Don't reset the pathlist of partitioned joinrels.

Hello,

On Fri, Dec 5, 2025 at 11:57 AM Robert Haas <robertmhaas@gmail.com> wrote:
> 014f9a831a320666bf2195949f41710f970c54ad removes the need for what was
> previously 0004, so here is a new patch series with that dropped, to
> avoid confusing cfbot or human reviewers.

I really like this idea! Telling the planner, "if you need to make a
decision for [this thing], choose [this way]," seems to be a really
nice way of sidestepping many of the concerns with "user control".

I've started an attempt to throw a fuzzer at this, because I'm pretty
useless when it comes to planner/optimizer review. I don't really know
what the overall fuzzing strategy is going to be, given the multiple
complicated inputs that have to be constructed and somehow correlated
with each other, but I'll try to start small and expand:

a) fuzz the parser first, because it's easy and we can get interesting inputs
b) fuzz the AST utilities, seeded with "successful" corpus members from a)
c) stare really hard at the corpus of b) and figure out how to
usefully mutate a PlannedStmt with it
d) use c) to fuzz pgpa_plan_walker, then pgpa_output_advice, then...?

I'm in the middle of an implementation of b) now, and it noticed the
following code (which probably bodes well for the fuzzer itself!):

>        if (rid->partnsp == NULL)
>            result = psprintf("%s/%s", result,
>                              quote_identifier(rid->partnsp));

I assume that should be quote_identifier(rid->partrel)?

= Other Notes =

GCC 11 complains about the following code in pgpa_collect_advice():

>        dsa_area   *area = pg_plan_advice_dsa_area();
>        dsa_pointer ca_pointer;
>
>        pgpa_make_collected_advice(userid, dbid, queryId, now,
>                                   query_string, advice_string, area,
>                                   &ca_pointer);
>        pgpa_store_shared_advice(ca_pointer);

It doesn't know that area is guaranteed to be non-NULL, so it can't
prove that ca_pointer is initialized.

(GCC also complains about unique_nonjoin_rtekind() not initializing
the rtekind, but I think that's because of a bug [1].)

--Jacob

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107838