Re: pg_plan_advice

From: "Dian Fay" <di(at)nmfay(dot)com>
To: "Robert Haas" <robertmhaas(at)gmail(dot)com>, "Matheus Alcantara" <matheusssilv97(at)gmail(dot)com>
Cc: "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-11-23 00:43:40
Message-ID: DEFNP3WX7BLX.1X7DS1P9X6JIF@nmfay.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue Nov 18, 2025 at 11:19 AM EST, Robert Haas wrote:
> Here's v4. This version has some bug fixes and test case changes to
> 0005 and 0006, with the goal of getting CI to pass cleanly (which it
> now does for me, but let's see if cfbot agrees).

Thanks for working on this, Robert! I think the design seems solid (and
very powerful) from a user perspective. I was curious what would happen
with row-level security interactions so I tried it out on a toy example
I put together a while back. I found one case where scan advice fails on
an intentionally naive/bad policy implementation, but I'm not sure why
and it seems like the kind of weird corner case that might be useful to
reason about. See attached for the setup script, then:

set pg_plan_advice.advice = 'BITMAP_HEAP_SCAN(item public.item_tags_idx)';
set item_reader.allowed_tags = '{alpha,beta}';
set role item_reader;

explain (plan_advice, analyze, verbose, costs, timing)
select * from item
where value ilike 'a%' and tags && array[1];

Seq Scan on public.item (cost=0.00..41777312.00 rows=54961 width=67) (actual time=2.947..8603.333 rows=6762.00 loops=1)
Disabled: true
Output: item.id, item.value, item.tags
Filter: (EXISTS(SubPlan exists_1) AND (item.value ~~* 'a%'::text) AND (item.tags && '{1}'::integer[]))
Rows Removed by Filter: 993238
Buffers: shared hit=1012312
SubPlan exists_1
-> Seq Scan on public.tag (cost=0.00..41.75 rows=1 width=0) (actual time=0.008..0.008 rows=0.21 loops=1000000)
Filter: ((current_setting('item_reader.allowed_tags'::text) IS NOT NULL) AND ((current_setting('item_reader.allowed_tags'::text))::text[] @> ARRAY[tag.name]) AND (item.tags @> ARRAY[tag.id]))
Rows Removed by Filter: 18
Buffers: shared hit=1000000
Planning Time: 1.168 ms
Supplied Plan Advice:
BITMAP_HEAP_SCAN(item public.item_tags_idx) /* matched, failed */
Generated Plan Advice:
SEQ_SCAN(item tag(at)exists_1)
NO_GATHER(item tag(at)exists_1)
Execution Time: 8603.615 ms

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

"Don't use this one index" sounds more fiddly to implement, but also
less valuable since in that case you probably already know which other
index it should be using.

Attachment Content-Type Size
rls-demo-item-tags.sql application/sql 1.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-11-23 00:44:41 Re: Inline non-SQL SRFs using SupportRequestSimplify
Previous Message Michael Paquier 2025-11-22 23:23:22 Re: Remove unused fields from BufferCacheNumaRec