MIME-Version: 1.0
References: 
 <CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com>
 <CAKZiRmxtJAFG7e1+Vs9B8ngON=AOzJbuws+1ZeH4LsbJh5AzoQ@mail.gmail.com>
 <CA+TgmoY9Ne_Sh10u6LSPc3wvOQPLp3kF9nBp3nqJEG2JGF2QiA@mail.gmail.com>
 <CA+Tgmoa57S6mP=aTOXH2-gDAL4TMO1WbGgrHSg0s6J4zUH=04g@mail.gmail.com>
 <DEB1SAGVDX7S.JMV96X4R0RCS@gmail.com>
 <CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
 <CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
 <DEFNP3WX7BLX.1X7DS1P9X6JIF@nmfay.com>
In-Reply-To: <DEFNP3WX7BLX.1X7DS1P9X6JIF@nmfay.com>
From: Robert Haas <robertmhaas@gmail.com>
Date: Mon, 24 Nov 2025 11:14:27 -0500
Message-ID: 
 <CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
Subject: Re: pg_plan_advice
To: Dian Fay <di@nmfay.com>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>,
 Jakub Wartak <jakub.wartak@enterprisedb.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BTgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi%3Dw%40mail.gmail.com>
Precedence: bulk

On Sat, Nov 22, 2025 at 7:43=E2=80=AFPM Dian Fay <di@nmfay.com> wrote:
> Thanks for working on this, Robert!

Thanks for looking at it! I was hoping for a bit more in the way of
responses by now, honestly.

> Since the policies don't contain any execution boundaries, all the quals
> should be going into a single bucket for planning if I understand the
> process correctly. The bitmap heap scan should be a candidate given the
> `tags &&` predicate (and indeed if I switch to a privileged role, the
> advice matches successfully without any policies in the mix), but gdb
> shows the walker bouncing out of pgpa_walker_contains_scan without any
> candidate scans for the BITMAP_HEAP_SCAN strategy.

I can understand why it seems that way, but when I try setting
enable_seqscan=3Dfalse instead of using pg_plan_advice, I get exactly
the same result. I think this is actually a great example both of why
this is actually a very powerful tool and also why it has the
potential to be really confusing. The power comes from the fact that
you can find out whether the planner thinks that the thing you want to
do is even possible. In this case, that's easy anyway because the
example is simple enough, but sometimes you can't set
enable_seqscan=3Dfalse or similar because it would change too many other
things in the plan at that same time and you wouldn't be able to
compare. In those situations, this figures to be useful. However, all
this can do is tell you that the answer to the question "is this a
possible plan shape?" is "no". It cannot tell you why, and you may
easily find the result counterintuitive.

And honestly, this is one of the things I'm worried about if we go
forward with this, that we'll get a ton of people who think it doesn't
work because it doesn't force the planner to do things which the
planner rejects on non-cost considerations. We're going to need really
good documentation to explain to people that if you use this to try to
force a plan and you can't, that's not a bug, that's the planner
telling you that that plan shape is not able to be considered for some
reason. That won't keep people from complaining about things that
aren't really bugs, but at least it will mean that there's a link we
can give them to explain why the way they're thinking about it is
incorrect. However, that will just beg the next question of WHY the
planner doesn't think a certain plan can be considered, and honestly,
I've found over the years that I often need to resort to the source
code to answer those kinds of questions. People who are not good at
reading C source code are not going to like that answer very much, but
I still think it's better if they know THAT the planner thinks the
plan shape is impossible even if we can't tell them WHY the planner
thinks that the plan shape is impossible. We probably will want to
document at least some of the common reasons why this happens, to cut
down on getting the same questions over and over again.

In this particular case, I think the problem is that the user-supplied
qual item.tags @> ARRAY[id] is not leakproof and therefore must be
tested after the security qual. There's no way to use a Bitmap Heap
Scan without reversing the order of those tests.

> I do want to avoid getting bikesheddy about the advice language so I'll
> forbear from syntax discussion, but one design thought with lower-level
> implications did occur to me as I was playing with this: it might be
> useful in some situations to influence the planner _away_ from known
> worse paths while leaving it room to decide on the best other option. I
> think the work you did in path management should make this pretty
> straightforward for join and scan strategies, since it looks like you've
> basically made the enable_* gucs a runtime-configurable bitmask (which
> seems like a perfectly reasonable approach to my "have done some source
> diving but not an internals hacker" eyes), and could disable one as
> easily as forcing one.

I mostly agree. Saying not to use a sequential scan on a certain
table, or not to use a particular index, or not to use a particular
join method seem like things that would be potentially useful, and
they would be straightforward generalizations of what the code already
does. For me, that would principally be a way to understand better why
the planner chose what it did. I often wonder what the planner's
second choice would have been, but I don't just want the plan with the
second-cheapest overall cost, because that will be something just
trivially different. I want the cheapest plan that excludes some key
element of the current plan, so I can see a meaningfully different
alternative.

That said, I don't see this being a general thing that would make
sense across all of the tags that pg_plan_advice supports. For
example, NO_JOIN_ORDER() sounds hard to implement and largely useless.

The main reason I haven't done this is that I want to keep the focus
on plan stability, or said differently, on things that can properly
round-trip. You should be able to run a query with EXPLAIN
(PLAN_ADVICE), then set pg_plan_advice.advice to the resulting string,
rerun the query, and get the same plan with all of the advice
successfully matching. Since EXPLAIN (PLAN_ADVICE) would never emit
these proposed negative tags, we'd need to think a little bit harder
about how that stuff should be tested. That's not necessarily a big
deal or anything, but I didn't think it was an essential element of
the initial scope, so I left it out. I'm happy to add it in at some
point, or for someone else to do so, but not until this much is
working well.

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