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

On Wed, Sep 9, 2026 at 7:16 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> The hard problem is finding #10 from Noah's original report, which is
> the case where you have two partitions of the same parent table that
> have the same name but are in different schemas, and you omit the
> schema name from the relation identifier. That is, you have a table,
> let's say foo, with partitions bar.foopart and baz.foopart. Instead of
> writing something like SEQ_SCAN(foo/bar.foopart) as the advice
> generator would do, you choose to write SEQ_SCAN(foo/foopart), making
> it ambiguous which child relation you're talking about. What happens
> right now is that advice enforcement will enforce that restriction
> against both children, but advice feedback will think that doesn't
> affect either one, so if you look at the advice feedback with EXPLAIN
> or print it out via pg_plan_advice.feedback_warnings, you'll see
> "matched, failed" instead of just "matched". Obviously, this kind of
> sucks. Of course, the blast radius is limited by the fact that almost
> nobody names their partitions this way, but theoretically they could
> and nobody likes features that work most of the time.
>
> Unfortunately, it is not at all straightforward to fix this. Pretty
> much everything the advice feedback code starts by mapping each
> relation identifier to an RTI, and giving up if the number of matches
> is not exactly 1. Since the whole purpose of relation identifiers is
> to map uniquely to relations appearing in the query, this is in
> general fine, but the case described in the previous paragraph breaks
> it. Generalizing the logic in the advice feedback code to be able to
> handle a relation identifier that maps to multiple RTIs looks like a
> bad idea. It would require massive adjustments to the code for a case
> that almost nobody has in real life, and the resulting code would be
> complicated and hard to understand and probably have corner cases
> where the performance is terrible. So I think something else has to
> give. A possible zero-order solution is to just document that advice
> feedback isn't guaranteed to work properly if you do this, so maybe
> you shouldn't. That is obviously not amazing but I think we could live
> with it for v19. What I'm thinking is probably better is to impose a
> restriction sufficient to keep this case from arising in the first
> place, such as:
>
> 1. Just rip out all the logic that allows the partition schema to be
> omitted, and require it always. This is only sad for people writing
> advice strings manually, since generated advice always includes the
> schema anyway.
>
> 2. Refuse to enforce advice if the partition schema is omitted and
> there's more than one partition with the same partition name. This
> changes the rule from "you can leave out the partition schema" to "you
> can leave out the partition schema when no ambiguity is thereby
> created," and it makes the existing behavior of the advice feedback
> system correct in retrospect. This seems like it would make just about
> nobody sad and therefore be just about perfect, except that I am not
> sure there's any way of implementing it that doesn't result in causing
> even bigger problems that the one it's trying to solve. More
> investigation needed.

What is your plan with cross- release compatibility for the mini
language and what advice strings are valid across releases? On some
level, idea 1 seems like the most appealing because a) it seems quick
to do before beta 4 b) it's the easiest to back down from if you find
a good way to support this later. If you don't require the schema and
later have to do that, you could have advice strings that no longer
work. Take this with a grain of salt, as I have neither used the
feature nor looked at its code :)

- Melanie