pg_plan_advice: add NO_ scan and join method tags

Hello,

Adding v2 as it v1 doesn't apply anymore.

Cheers,
Florin
www.enterprisedb.com

Hi Florin,
I tested the patch on current master, and it worked well in my testing.
Before applying the patch, advice such as NO_SEQ_SCAN(t1) was rejected
with a parser error because the NO_ scan and join tags were not
supported.
After applying the patch, the new NO_ tags were accepted successfully.
I verified that:
NO_SEQ_SCAN prevented the planner from using a Sequential Scan when
another eligible scan method was available.
NO_HASH_JOIN caused the planner to choose an alternative join method.
Multiple NO_ advice tags worked as expected.
Conflicting positive and negative advice for the same method was
detected correctly.
Positive and negative advice for different methods worked together as expected.
I also ran the regression tests, and everything completed successfully.

Thanks for working on this enhancement.

Regards

solai

Hello!

Hello,

The attached patch adds negative scan- and join-method advice to
pg_plan_advice. These tags are the complement of the existing
positive ones:

I was trying to measure the test coverage of this patch and I found that
the meson configuration wasn't updated, I think you're missing something
like this:

--- a/contrib/pg_plan_advice/meson.build
+++ b/contrib/pg_plan_advice/meson.build
@@ -57,6 +57,8 @@ tests += {
       'gather',
       'join_order',
       'join_strategy',
+      'no_join',
+      'no_scan',
       'partitionwise',
       'prepared',
       'scan',

After the tests run only two cases were not tested NO_MERGE_JOIN and
NO_NESTED_LOOP, but these supposed to be tested, well I'll dig more on
this one a second pass of the review.

During the check, this came to my attention:

+               case PGPA_TAG_NO_INDEX_ONLY_SCAN:
+                       return PGS_INDEXONLYSCAN | PGS_CONSIDER_INDEXONLY;

Reading the comments I got this:

When PGS_CONSIDER_INDEXONLY is
unset, we don't even consider index-only scans, and any such scans that
would have been generated become index scans instead. On the other hand,
unsetting PGS_INDEXSCAN or PGS_INDEXONLYSCAN causes generated paths of the
corresponding types to be marked as disabled.

The message is a bit confusing but, I think that
`PGS_INDEXONLY` should be the only option here? Otherwise, my
understanding is that it will disable also PGS_INDEXSCAN, which is in
the next line to be disable.

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

Hello,

On 14/07/2026 14:19, Jonathan Gonzalez V. wrote:

I was trying to measure the test coverage of this patch and I found that
the meson configuration wasn't updated, I think you're missing something
like this:

--- a/contrib/pg_plan_advice/meson.build
+++ b/contrib/pg_plan_advice/meson.build
@@ -57,6 +57,8 @@ tests += {
'gather',
'join_order',
'join_strategy',
+      'no_join',
+      'no_scan',
'partitionwise',
'prepared',
'scan',

Thanks for catching the meson.build gap — confirmed, no_join/no_scan
were added to the Makefile's REGRESS list but never mirrored into
meson.build. Fixed.

After the tests run only two cases were not tested NO_MERGE_JOIN and
NO_NESTED_LOOP, but these supposed to be tested, well I'll dig more on
this one a second pass of the review.

On the coverage gap for NO_MERGE_JOIN/NO_NESTED_LOOP: I instrumented
both switch cases in pgpa_walker.c with log probes and reran no_join.sql
— both fire. Concretely: NO_NESTED_LOOP(f) (sql/no_join.sql:38) and the
stacked NO_HASH_JOIN(f) NO_NESTED_LOOP(f) case (line 52) both hit the
PGPA_TAG_NO_NESTED_LOOP case; NO_MERGE_JOIN(d) (line 45) hits
PGPA_TAG_NO_MERGE_JOIN. So ISTM the underlying advice-tag handling is
exercised.

During the check, this came to my attention:

+               case PGPA_TAG_NO_INDEX_ONLY_SCAN:
+                       return PGS_INDEXONLYSCAN | PGS_CONSIDER_INDEXONLY;

Reading the comments I got this:

When PGS_CONSIDER_INDEXONLY is
unset, we don't even consider index-only scans, and any such scans that
would have been generated become index scans instead. On the other hand,
unsetting PGS_INDEXSCAN or PGS_INDEXONLYSCAN causes generated paths of the
corresponding types to be marked as disabled.

The message is a bit confusing but, I think that
`PGS_INDEXONLY` should be the only option here? Otherwise, my
understanding is that it will disable also PGS_INDEXSCAN, which is in
the next line to be disable.

On PGS_CONSIDER_INDEXONLY: I think this traces back to the pathnodes.h
comment:

▎ When PGS_CONSIDER_INDEXONLY is unset, we don't even consider
index-only scans, and any such scans that would have been generated
become index scans instead. On the other hand, unsetting PGS_INDEXSCAN
or PGS_INDEXONLYSCAN causes generated paths of the corresponding types
to be marked as disabled.

That comment is actually describing the behavior we want.
NO_INDEX_ONLY_SCAN's mask clears PGS_INDEXONLYSCAN |
PGS_CONSIDER_INDEXONLY, leaving PGS_INDEXSCAN untouched. Per the
comment, clearing PGS_CONSIDER_INDEXONLY doesn't disable a path — it
changes what check_index_only() builds in the first place, converting
the would-be Index-Only Scan into a regular Index Scan path. Since we
never touch PGS_INDEXSCAN, that converted path stays enabled. Drop
PGS_CONSIDER_INDEXONLY from the mask (your suggestion) and
check_index_only() still returns true, so the planner builds an actual
(disabled) Index-Only Scan with no regular Index Scan alternative for
that index at all.

I confirmed this with the mask reduced to just PGS_INDEXONLYSCAN, the
no_scan test that expects NO_INDEX_ONLY_SCAN to fall back to a plain
Index Scan instead produced a Bitmap Heap Scan. So ISTM the current code
is correct.

attaching v3 with the meson.build change and rebased on current master.

Cheers,
Florin
---
www.enterprisedb.com

At 2026-07-28 00:45:08, "Florin Irion" <irionr@gmail.com> wrote:

On PGS_CONSIDER_INDEXONLY: I think this traces back to the pathnodes.h
comment:

When PGS_CONSIDER_INDEXONLY is unset, we don't even consider
index-only scans, and any such scans that would have been generated
become index scans instead. On the other hand, unsetting PGS_INDEXSCAN
or PGS_INDEXONLYSCAN causes generated paths of the corresponding types
to be marked as disabled.

That comment is actually describing the behavior we want.
NO_INDEX_ONLY_SCAN's mask clears PGS_INDEXONLYSCAN |
PGS_CONSIDER_INDEXONLY, leaving PGS_INDEXSCAN untouched. Per the
comment, clearing PGS_CONSIDER_INDEXONLY doesn't disable a path — it
changes what check_index_only() builds in the first place, converting
the would-be Index-Only Scan into a regular Index Scan path. Since we
never touch PGS_INDEXSCAN, that converted path stays enabled. Drop
PGS_CONSIDER_INDEXONLY from the mask (your suggestion) and
check_index_only() still returns true, so the planner builds an actual
(disabled) Index-Only Scan with no regular Index Scan alternative for
that index at all.

I confirmed this with the mask reduced to just PGS_INDEXONLYSCAN, the
no_scan test that expects NO_INDEX_ONLY_SCAN to fall back to a plain
Index Scan instead produced a Bitmap Heap Scan. So ISTM the current code
is correct.

attaching v3 with the meson.build change and rebased on current master.

Hi,
I have a minor comment on the v3 patch.

diff --git a/contrib/pg_plan_advice/pgpa_planner.c b/contrib/pg_plan_advice/pgpa_planner.c
index b3329b793aa..a29500e0e1d 100644
--- a/contrib/pg_plan_advice/pgpa_planner.c
+++ b/contrib/pg_plan_advice/pgpa_planner.c

+           /* Handle NO_ join method advice. */
+           {
+                 uint64            my_no_join_mask;
+
+                 my_no_join_mask = pgpa_no_join_mask_from_advice_tag(entry->tag);
+                 if (my_no_join_mask != 0)
+                 {
+                       bool        permit;
+                       bool        restrict_method;
+
+                       /*
+                        * NO_ join tags impose the same join-order constraint as
+                        * positive ones: the target must be the inner rel.  Reuse
+                        * pgpa_join_method_permits_join to enforce it.
+                        */
+                       permit = pgpa_join_method_permits_join(pjs->outer_count,
+                                                                                pjs->inner_count,
+                                                                                pjs->rids,
+                                                                                entry,
+                                                                                &restrict_method);
+                       if (!permit)
+                             jo_deny_indexes = bms_add_member(jo_deny_indexes, i);
+                       else if (restrict_method)
+                       {
+                             no_jm_indexes = bms_add_member(no_jm_indexes, i);
+                             no_join_mask |= my_no_join_mask;
+                       }
+                       continue;
+                 }
+           }

Regarding the semantics of NO_ tags:
NO_HASH_JOIN((a b)) means a hash join cannot be used when the join product of a and b appears on the inner side.

Consider the following scenario:
SET pg_plan_advice.advice = 'JOIN_ORDER(t4 ((t2 t3) t1)) NO_HASH_JOIN((t1 t2))';

pgpa_join_method_permits_join() matches the set of inner relations against the target.
The result is ITM_TARGETS_ARE_SUBSET and restrict_method=false.
When inner={a,b,c}, a and b are indeed together on the inner side as part of a larger join product.
Per the intended semantics, the restriction from the NO_ tag should take effect: a hash join would be used with an inner side containing {a,b}.

But the current implementation only enforces the constraint for ITM_EQUAL and skips the ITM_TARGETS_ARE_SUBSET case.

I think this may be an issue.

Yanli Song

Hi,

On 28/07/2026 08:43, song yanli wrote:

Regarding the semantics of NO_ tags:
NO_HASH_JOIN((a b)) means a hash join cannot be used when the join
product of a and b appears on the inner side.

Consider the following scenario:
SET pg_plan_advice.advice = 'JOIN_ORDER(t4 ((t2 t3) t1))
NO_HASH_JOIN((t1 t2))';

pgpa_join_method_permits_join() matches the set of inner relations
against the target.
The result is ITM_TARGETS_ARE_SUBSET and restrict_method=false.
When inner={a,b,c}, a and b are indeed together on the inner side as
part of a larger join product.
Per the intended semantics, the restriction from the NO_ tag should
take effect: a hash join would be used with an inner side containing
{a,b}.

But the current implementation only enforces the constraint for
ITM_EQUAL and skips the ITM_TARGETS_ARE_SUBSET case.

I tested this using  JOIN_ORDER(t4 (t3 (t1 t2)))  with and without
NO_HASH_JOIN((t1 t2)):

- Where the inner side is exactly {t1,t2} (ITM_EQUAL), the tag fires:
  Hash Join → Merge Join there.
- Where the inner side is {t1,t2,t3} (ITM_TARGETS_ARE_SUBSET), the Hash Join
  stays, and EXPLAIN (PLAN_ADVICE) reports the tag as /* matched */.

Per the docs, HASH_JOIN((a b))/NO_HASH_JOIN((a b)) both refer to the join
product of exactly a and b — not any join whose inner side happens to
contain
them plus other relations. pgpa_join_method_permits_join() is shared
betwen the
positive and negative tags precisely to keep "NO_ is the logical
complement of
the positive tag" true; enforcing the NO_ form more broadly tahn the
positive
form's own matching scope would break that symmetry. The code comment
says it
directly: for the TARGETS_ARE_SUBSET case, "HASH_JOIN((x y)) doesn't
restrict
how x and y can be joined" — the join event being controlled is specifically
the one where {a,b} becomes someone else's inner side, not any join that
merely
contains them.
On your exact example, JOIN_ORDER(t4 ((t2 t3) t1)) NO_HASH_JOIN((t1 t2)):
this isn't a silent bypass. That specific order requires joining t1 to
the already
combined (t2 t3), which splits the {t1,t2} target across sides (t2
merges with
outsider t3 before t1 and t2 join each other), so NO_HASH_JOIN's own
join-order
logic denies that pairing, right where JOIN_ORDER demands it. Both tags come
back marked conflicting in the advice output, and the permit wins, so
the plan
still follows your requested order.

I think this may be an issue.

If you think this should be changed I think it's a design change that
should be
discussed on a separate thread, what do you think?

Cheers,
Florin
www.enterprisedb.com

Hi Florin,

While testing v3, I found two behaviors that look unintended.

1. False conflict between BITMAP_HEAP_SCAN and NO_INDEX_ONLY_SCAN

This is separate from the earlier discussion about whether
NO_INDEX_ONLY_SCAN should clear PGS_CONSIDER_INDEXONLY. I agree that it
needs to do so for enforcement. The problem is that the same bit is also
used for conflict detection.

BITMAP_HEAP_SCAN uses:

PGS_BITMAPSCAN | PGS_CONSIDER_INDEXONLY

while NO_INDEX_ONLY_SCAN forbids:

PGS_INDEXONLYSCAN | PGS_CONSIDER_INDEXONLY

Therefore this check reports a conflict due only to the shared
PGS_CONSIDER_INDEXONLY bit:

if (scan_type != all_scan_mask && (scan_type & no_scan_mask) != 0)
scan_pos_neg_conflict = true;

For example:

SET LOCAL pg_plan_advice.advice =
'BITMAP_HEAP_SCAN(no_scan_table)
NO_INDEX_ONLY_SCAN(no_scan_table)';

EXPLAIN (COSTS OFF, PLAN_ADVICE)
SELECT * FROM no_scan_table WHERE a = 1;

reports:

BITMAP_HEAP_SCAN(no_scan_table)
/* matched, conflicting, failed */
NO_INDEX_ONLY_SCAN(no_scan_table)
/* matched, conflicting */

These tags are semantically compatible: a Bitmap Heap Scan is not an Index
Only Scan. It seems that PGS_CONSIDER_INDEXONLY is an enforcement detail,
not a scan method for conflict purposes.

Would it make sense to use separate semantic and enforcement masks, with
only the former used for conflict detection?

2. A conflict cancels unrelated negative tags

A single scan_pos_neg_conflict or jm_pos_neg_conflict boolean is used for
all negative tags on the target. Once set, every negative tag is marked
conflicting and the entire no_scan_mask or no_join_mask is not applied.

For example:

SET LOCAL pg_plan_advice.advice =
'SEQ_SCAN(no_scan_table)
NO_SEQ_SCAN(no_scan_table)
NO_BITMAP_HEAP_SCAN(no_scan_table)';

produces:

Bitmap Heap Scan on no_scan_table
Recheck Cond: (b > 'some text 8'::text)
-> Bitmap Index Scan on no_scan_table_b
Index Cond: (b > 'some text 8'::text)
Supplied Plan Advice:
SEQ_SCAN(no_scan_table) /* matched, conflicting, failed */
NO_SEQ_SCAN(no_scan_table) /* matched, conflicting */
NO_BITMAP_HEAP_SCAN(no_scan_table) /* matched, conflicting, failed */

The SEQ_SCAN/NO_SEQ_SCAN pair conflicts, but NO_BITMAP_HEAP_SCAN does not.
Nevertheless, it is also marked conflicting and is not enforced, allowing
the final plan to use a Bitmap Heap Scan.

The join case behaves the same way:

SET LOCAL pg_plan_advice.advice =
'MERGE_JOIN_PLAIN(d)
NO_MERGE_JOIN(d)
NO_HASH_JOIN(d)';

produces:

Hash Join
Hash Cond: (f.dim_id = d.id)
-> Seq Scan on no_join_fact f
-> Hash
-> Seq Scan on no_join_dim d
Supplied Plan Advice:
MERGE_JOIN_PLAIN(d) /* matched, conflicting, failed */
NO_MERGE_JOIN(d) /* matched, conflicting */
NO_HASH_JOIN(d) /* matched, conflicting, failed */

NO_HASH_JOIN(d) is marked conflicting and left unenforced even though it
does not conflict with MERGE_JOIN_PLAIN(d), so the final plan can still use
a Hash Join.

Would it be better to detect conflicts per method, mark only the involved
tags as conflicting, and continue applying the enforcement masks of
unrelated negative tags?

Regression tests for both combinations would be useful.

Regards,
--
Ze Chen (Neil)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 29/07/2026 09:12, Neil Chen wrote:

Hi Florin,

While testing v3, I found two behaviors that look unintended.

1. False conflict between BITMAP_HEAP_SCAN and NO_INDEX_ONLY_SCAN

This is separate from the earlier discussion about whether
NO_INDEX_ONLY_SCAN should clear PGS_CONSIDER_INDEXONLY. I agree that it
needs to do so for enforcement. The problem is that the same bit is also
used for conflict detection.

BITMAP_HEAP_SCAN uses:

|PGS_BITMAPSCAN | PGS_CONSIDER_INDEXONLY|

while NO_INDEX_ONLY_SCAN forbids:

|PGS_INDEXONLYSCAN | PGS_CONSIDER_INDEXONLY|

Therefore this check reports a conflict due only to the shared
PGS_CONSIDER_INDEXONLY bit:

|if (scan_type != all_scan_mask && (scan_type & no_scan_mask) != 0)
scan_pos_neg_conflict = true;|

For example:

|SET LOCAL pg_plan_advice.advice = 'BITMAP_HEAP_SCAN(no_scan_table)
NO_INDEX_ONLY_SCAN(no_scan_table)'; EXPLAIN (COSTS OFF, PLAN_ADVICE)
SELECT * FROM no_scan_table WHERE a = 1;|

reports:

|BITMAP_HEAP_SCAN(no_scan_table) /* matched, conflicting, failed */
NO_INDEX_ONLY_SCAN(no_scan_table) /* matched, conflicting */|

These tags are semantically compatible: a Bitmap Heap Scan is not an Index
Only Scan. It seems that PGS_CONSIDER_INDEXONLY is an enforcement detail,
not a scan method for conflict purposes.

Would it make sense to use separate semantic and enforcement masks, with
only the former used for conflict detection?

2. A conflict cancels unrelated negative tags

A single scan_pos_neg_conflict or jm_pos_neg_conflict boolean is used for
all negative tags on the target. Once set, every negative tag is marked
conflicting and the entire no_scan_mask or no_join_mask is not applied.

For example:

|SET LOCAL pg_plan_advice.advice = 'SEQ_SCAN(no_scan_table)
NO_SEQ_SCAN(no_scan_table) NO_BITMAP_HEAP_SCAN(no_scan_table)';

produces:

|Bitmap Heap Scan on no_scan_table Recheck Cond: (b > 'some text
8'::text) -> Bitmap Index Scan on no_scan_table_b Index Cond: (b >
'some text 8'::text) Supplied Plan Advice: SEQ_SCAN(no_scan_table) /*
matched, conflicting, failed */ NO_SEQ_SCAN(no_scan_table) /* matched,
conflicting */ NO_BITMAP_HEAP_SCAN(no_scan_table) /* matched,
conflicting, failed */||

The SEQ_SCAN/NO_SEQ_SCAN pair conflicts, but NO_BITMAP_HEAP_SCAN does not.
Nevertheless, it is also marked conflicting and is not enforced, allowing
the final plan to use a Bitmap Heap Scan.

The join case behaves the same way:

|SET LOCAL pg_plan_advice.advice = 'MERGE_JOIN_PLAIN(d)
NO_MERGE_JOIN(d) NO_HASH_JOIN(d)';

produces:

|Hash Join Hash Cond: (f.dim_id = d.id <http://d.id&gt;) -> Seq Scan on
no_join_fact f -> Hash -> Seq Scan on no_join_dim d Supplied Plan
Advice: MERGE_JOIN_PLAIN(d) /* matched, conflicting, failed */
NO_MERGE_JOIN(d) /* matched, conflicting */ NO_HASH_JOIN(d) /*
matched, conflicting, failed */||

NO_HASH_JOIN(d) is marked conflicting and left unenforced even though it
does not conflict with MERGE_JOIN_PLAIN(d), so the final plan can
still use
a Hash Join.

Would it be better to detect conflicts per method, mark only the involved
tags as conflicting, and continue applying the enforcement masks of
unrelated negative tags?

Regression tests for both combinations would be useful.

Hi,

Thank you for reviewing,all confirmed, and fixed in v4.

1. False conflict via PGS_CONSIDER_INDEXONLY: that's an enforcement
detail, not a scan method. Added pgpa_scan_semantic_mask(), which strips
PGS_CONSIDER_INDEXONLY before comparing tags for conflict purposes.
BITMAP_HEAP_SCAN(t) NO_INDEX_ONLY_SCAN(t) now comes back clean, no conflict.

2. Conflict cancelling unrelated NO_ tags: Replaced the single
all-or-nothing scan_pos_neg_conflict/jm_pos_neg_conflict gate with
per-tag conflict tracking. Each NO_ tag is now checked independently;
only the ones that actually clash get marked conflicting and skipped,
everything else still gets enforced. Verified with your examples —
NO_BITMAP_HEAP_SCAN and NO_HASH_JOIN(d) now apply despite an unrelated
conflict on a different method.

3. Also caught while testing: index-selection side effect on conflict.
Separately, found that the "enforce choice of index" block only checked
!scan_pos_conflict, not !scan_pos_neg_conflict. So INDEX_SCAN(t idx)
NO_INDEX_SCAN(t) — a genuine conflict, correctly left unenforced at the
mask level — was still silently disabling every other index on t,
steering the plan toward idx anyway. Fixed by adding the missing check.

Hi Florin,

On Thu, Jul 30, 2026 at 4:55 PM Florin Irion <irionr@gmail.com> wrote:

Hi,

Thank you for reviewing,all confirmed, and fixed in v4.

1. False conflict via PGS_CONSIDER_INDEXONLY: that's an enforcement
detail, not a scan method. Added pgpa_scan_semantic_mask(), which strips
PGS_CONSIDER_INDEXONLY before comparing tags for conflict purposes.
BITMAP_HEAP_SCAN(t) NO_INDEX_ONLY_SCAN(t) now comes back clean, no
conflict.

2. Conflict cancelling unrelated NO_ tags: Replaced the single
all-or-nothing scan_pos_neg_conflict/jm_pos_neg_conflict gate with
per-tag conflict tracking. Each NO_ tag is now checked independently;
only the ones that actually clash get marked conflicting and skipped,
everything else still gets enforced. Verified with your examples —
NO_BITMAP_HEAP_SCAN and NO_HASH_JOIN(d) now apply despite an unrelated
conflict on a different method.

3. Also caught while testing: index-selection side effect on conflict.
Separately, found that the "enforce choice of index" block only checked
!scan_pos_conflict, not !scan_pos_neg_conflict. So INDEX_SCAN(t idx)
NO_INDEX_SCAN(t) — a genuine conflict, correctly left unenforced at the
mask level — was still silently disabling every other index on t,
steering the plan toward idx anyway. Fixed by adding the missing check.

Thanks for v4. I've tested it and everything looks correct -- all three
issues
fixed, and the index-selection side effect is handled nicely too. +1 from
me.

Regards,
--
Ze Chen (Neil)
HighGo Software Co., Ltd.
https://www.highgo.com/