On Mon, Dec 29, 2025 at 6:34 PM Haibo Yan <tristan.yim@gmail.com> wrote:
> 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
orderor path that is being enforced by the advice mask? In that case, could this lead to failures such as inability to
constructthe required join relation or excessive planning time if the desired path is not sampled?
The interaction of this feature with GEQO definitely needs more study.
If you have some time to work on this, I think testing and reporting
results would be quite useful. However, I don't think we should ever
get planner failure, and I'm doubtful about excessive planning time as
well. The effect of plan advice is to disable some paths just as if
enable_<whatever> were set to false, so if you provide very specific
advice while planning with GEQO, I think you might just end up with a
disabled path that doesn't account for the advice. However, this
should be checked, and I haven't gotten there yet. I'll add an XXX to
the README to make sure this doesn't get forgotten.
> 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
correspondingchanges in outfuncs.c / readfuncs.c. Without serialization support, parallel workers executing subplans or
Appendnodes may not receive this metadata. Is this handled elsewhere, or is it something still pending?
I believe that gen_node_support.pl should take care of this
automatically unless the node type is flagged as
pg_node_attr(custom_read_write).
> 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
underlyingtable 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?
No. That function is only used for indexes.
> 4. Minor typo (patch 4):
> In src/include/nodes/relation.h, parititonwise appears to be a typo and should likely be partitionwise.
Will fix, thanks.
--
Robert Haas
EDB: http://www.enterprisedb.com
