| From: | Ajay Pal <ajay(dot)pal(dot)k(at)gmail(dot)com> |
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(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>, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
| Subject: | Re: pg_plan_advice |
| Date: | 2025-12-15 06:30:43 |
| Message-ID: | CABRHmyvxbRqkK82KjQ5B=yvN+Y+TTh=UG6qL4yrXbf9zCwSHTw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
During further testing of the plan_advice patch's latest version, I
observed that the following query is generating a no_gather plan. This
specific plan structure is not being accepted by the query planner.
postgres=*# set local pg_plan_advice.advice='NO_GATHER("*RESULT*")';
SET
postgres=*# explain ( plan_advice) SELECT
CAST('99999999999999999999999999999999999999999999999999' AS NUMERIC);
QUERY PLAN
-------------------------------------------
Result (cost=0.00..0.01 rows=1 width=32)
Supplied Plan Advice:
NO_GATHER("*RESULT*") /* not matched */
Generated Plan Advice:
NO_GATHER("*RESULT*")
(5 rows)
Thanks
Ajay
On Fri, Dec 12, 2025 at 11:40 PM Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>
> On Fri, Dec 12, 2025 at 9:36 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > 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.
>
> Makes sense (this fuzzer was exercising pgpa_format_advice_target()).
>
> > > 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?
>
> Yeah.
>
> Next bit of fuzzer feedback: I need the following diff in
> pgpa_trove_add_to_hash() to avoid a crash when the hashtable starts to
> fill up:
>
> > element = pgpa_trove_entry_insert(hash, key, &found);
> > + if (!found)
> > + element->indexes = NULL;
> > element->indexes = bms_add_member(element->indexes, index);
>
> The advice string that triggered this is horrific, but I can send it
> to you offline if you're morbidly curious. (I can spend time to
> minimize it or I can get more fuzzer coverage, and I'd rather do the
> latter right now :D)
>
> --Jacob
>
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Laurenz Albe | 2025-12-15 06:49:03 | Re: Get rid of "Section.N.N.N" on DOCs |
| Previous Message | Bryan Green | 2025-12-15 06:03:44 | Re: [PATCH] Allow complex data for GUC extra. |