Re: pg_plan_advice: add NO_ scan and join method tags

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/