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