Re: pg_plan_advice

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: Dian Fay <di(at)nmfay(dot)com>, Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_plan_advice
Date: 2025-12-12 17:36:17
Message-ID: CA+TgmoaX2AMW4cdFM3OngBJxmxpkdmzF33R7-CWhvRLfucbFMg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 11, 2025 at 8:11 PM Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
> Sure! (They'll need to be golfed down.) Here are three entries that
> hit the crash, each on its own line:
>
> > join_order(qoe((nested_l oindex_scanp_plain))se(nested_loop_plain)nested_loo/_pseq_scanlain)
> > join_order(qoe((nested_loop_plain))se(nested_loop_plain)nesemij/insted_loop_plain)
> > gather(gather(gar(g/ther0))gtaher(gathethga))

At least for me, setting pg_plan_advice.advice to any of these strings
does not provoke a crash. What I discovered after a bit of
experimentation is that you get the crash if you (a) set the string to
something like this and then (b) run an EXPLAIN. Turns out, I already
had a test in syntax.sql that is sufficient to provoke the crash, so,
locally, I've added 'EXPLAIN SELECT 1' after each test case in
syntax.sql that is expected to successfully alter the value of the
GUC.

> Something the fuzzer really likes is zero-length identifiers ("").
> Maybe that's by design, but I thought I'd mention it since the
> standard lexer doesn't allow that and syntax.sql doesn't exercise it.

That's not by design. I've added a matching error check locally.

> > > It doesn't know that area is guaranteed to be non-NULL, so it can't
> > > prove that ca_pointer is initialized.
> >
> > I don't know what to do about that. I can understand why it might be
> > unable to prove that, but I don't see an obvious way to change the
> > code that would make life easier. I could add Assert(area != NULL)
> > before the call to pgpa_make_collected_advice() if that helps.
>
> With USE_ASSERT_CHECKING, that should help, but I'm not sure if it
> does without. (I could have sworn there was a conversation about that
> at some point but I can't remember any of the keywords.) Could also
> just make a dummy assignment. Or tag pg_plan_advice_dsa_area() with
> __attribute__((returns_nonnull)), but that's more portability work.

As in initialize ca_pointer to InvalidDsaPointer?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Paul A Jungwirth 2025-12-12 17:48:48 Re: Inval reliability, especially for inplace updates
Previous Message Sergey Soloviev 2025-12-12 16:23:15 Re: Introduce Index Aggregate - new GROUP BY strategy