Re: pg_plan_advice: add NO_ scan and join method tags

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