Re: pg_plan_advice

Hi Robert,

Thank you very much for your work on the pg_plan_advice patch series. It is
an impressive and substantial contribution, and it seems like a meaningful
step forward toward addressing long-standing query plan stability issues in
PostgreSQL.

While reviewing the v7 patches, I noticed a few points that I wanted to
raise for discussion:
1. GEQO interaction (patch 4):
Since GEQO relies on randomized search, is there a risk that the optimizer
may fail to explore the specific join order or path that is being enforced
by the advice mask? In that case, could this lead to failures such as
inability to construct the required join relation or excessive planning
time if the desired path is not sampled?
2. Parallel query serialization (patches 1–3):
Several new fields (subrtinfos, elidedNodes, child_append_relid_sets) are
added to PlannedStmt, but I did not see corresponding changes in outfuncs.c
/ readfuncs.c. Without serialization support, parallel workers executing
subplans or Append nodes may not receive this metadata. Is this handled
elsewhere, or is it something still pending?
3. Alias handling when generating advice (patch 5):
In pgpa_output_relation_name, the advice string is generated using
get_rel_name(relid), which resolves to the underlying table name rather
than the RTE alias. In self-join cases this could be ambiguous (e.g.,
my_table vs my_table). Would it be more appropriate to use the RTE alias
when available?
4. Minor typo (patch 4):
In src/include/nodes/relation.h, parititonwise appears to be a typo and
should likely be partitionwise.

I hope these comments are helpful, and I apologize in advance if any of
this is already addressed elsewhere in the series.

Best regards,
Haibo

On Mon, Dec 15, 2025 at 12:06 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> Here's v7.
>
> In 0001, I removed "const" from a node's struct declaration, because
> Tom gave me some feedback to avoid that on another recent patch, and I
> noticed I had done it here also. 0002, 0003, and 0004 are unchanged.
>
> In 0005:
>
> - Refactored the code to avoid issuing SEMIJOIN_NON_UNIQUE() advice in
> cases where uniqueness wasn't actually considered.
> - Adjusted the code not to issue NO_GATHER() advice for non-relation
> RTEs. (This is the issue reported by Ajay Pal in a recent message to
> this thread, which was also mentioned in an XXX in the code.)
> - Reject zero-length delimited identifiers, per Jacob's email.
> - Properly initialize element->indexes in pgpa_trove_add_to_hash, per
> Jacob'e email.
> - Add gather((d d/d.d)) test case, per Jacob, and fix the related bug
> in pgpa_identifier_matches_target, per Jacob's email.
> - Add EXPLAIN SELECT 1 after various test cases in syntax.sql, to
> improve test coverage, per analysis of why the existing test case
> didn't catch a bug previously reported by Jacob.
> - Added a dummy initialization to pgpa_collector.c to placate nervous
> compilers, per discussion with Jacob.
>
> I think this mostly catches me up on responding to issues reported
> here, although there is one thing reported to me off-list that I
> haven't dealt with yet. If there's anything reported on thread that
> isn't addressed here, let me know.
>
> Thanks,
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>