On Mon, Dec 29, 2025 at 6:34 PM Haibo Yan <[email protected]> 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 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?
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 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? 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 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? 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
