Re: pg_*_advice: tsv load failure, etc.

Hi,

On Thu, 10 Sept 2026 at 23:43, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Sep 10, 2026 at 10:59 AM Nathan Bossart
> <nathandbossart(at)gmail(dot)com> wrote:
> > I agree with Melanie. Idea 1 seems like the obvious way to go for v19,
> > especially at this stage of the game. In fact, I think there's a good
> > argument for keeping it that way long term; I see no benefit to inflicting
> > more search_path-style issues unless it provides some huge amount of
> > convenience that we simply cannot pass up.
>
> Sounds like a consensus. Here's a new patch set (v5) developed with
> that in mind. I've pushed the previous 0001, which was the
> documentation fix for GEQO vs. pg_plan_advice, plus a fix for pgindent
> breakage reported to me off-list. So now we have:
>
> 0001 Change GEQO fitness comparisons to consider disabled_nodes:
> Nathan mentioned off-list that it would be good to judge the blast
> radius of this change, since it is an ABI break. Although it's a bug
> in v18, I only plan to backpatch to v19, so I think it's OK
> regardless, but codesearch.debian.net didn't turn up any meaningful
> hits for geqo_eval. Is there something else I should be looking at?
>
> 0002 Fix defects in JOIN_ORDER advice feedback. Could use review, but
> probably won't get anybody, since nobody looks at my code. I think
> it's fine, though.

I applied v5 and spent some time looking at 0002. ISTM there may still be
one nearby case that isn't handled:

JOIN_ORDER((a b))
JOIN_ORDER({a b})

AFAICS, both forms are enforced correctly, but the feedback says
"partially matched". With feedback_warnings enabled, the first case also
produces:

WARNING: supplied plan advice was not enforced
DETAIL: advice JOIN_ORDER((a b)) feedback is "partially matched"

I think this happens because pgpa_join_order_permits_join() descends into
the only child and treats it as a sublist, so it never records a full match.

Would it make sense to cover these cases in 0002 as well, or is the
different feedback intentional?

> 0003 Disallow empty sublists within JOIN_ORDER(). New in this version
> of the patch set. Trivial fix for a gap in parse-time validation of
> advice strings.

0003 looks good to me.

Regards,
Ayush