At 2026-07-28 00:45:08, "Florin Irion" <[email protected]> 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
