Re: pg_plan_advice
Robert Haas <robertmhaas@gmail.com>
Attachments
- v16-0002-Replace-get_relation_info_hook-with-build_simple.patch (application/octet-stream) patch v16-0002
- v16-0005-Consider-startup-cost-as-a-figure-of-merit-for-p.patch (application/octet-stream) patch v16-0005
- v16-0004-Fix-add_partial_path-interaction-with-disabled_n.patch (application/octet-stream) patch v16-0004
- v16-0001-Allow-extensions-to-mark-an-individual-index-as-.patch (application/octet-stream) patch v16-0001
- v16-0006-Test-pg_plan_advice-using-a-new-test_plan_advice.patch (application/octet-stream) patch v16-0006
- v16-0003-Add-pg_plan_advice-contrib-module.patch (application/octet-stream) patch v16-0003
On Tue, Feb 10, 2026 at 6:03 PM Robert Haas <robertmhaas@gmail.com> wrote: > I have committed those, as well as 0001 and 0002. Here's v15. Here's v16. As a note, my biggest need right now is for some code review. Testing help is, of course, also still very welcome. What I have done in this version is run the test_plan_advice machinery with the pg_plan_advice.feedback_warnings=on and try to fix as many of the resulting failures as I could. The biggest revelation that I had as a result of doing that is that it's really unfortunate for this patch that get_relation_info_hook is only called for RTE_RELATION baserels. I was somewhat aware of this problem before, but I thought that it might be a non-critical thing that could be fixed at some indefinite future point, maybe years in the future. However, this testing convinced me that this could not really be postponed if I wanted the patch set to actually work as intended. It's true that there's nothing to control in terms of scan method for something like a subquery scan, but you can want to control the placement of or prevent the occurrence of Gather or Gather Merge nodes in join problems that involve non-relation RTEs, and there's really no way to do that with the existing hook placement. But it seems like there's a pretty easy solution, which is to move the hook up from get_relation_info to its sole caller, build_simple_rel. Patch 0002 implements this and has a commit message explaining how code that currently uses get_relation_info_hook can be modified to work with the replacement, buld_simple_rel_hook. The rest of the patch set is as before, except for the main patch, now 0003, which has the following additional changes: - It has been updated to use build_simple_rel_hook rather than get_relation_info_hook. - I have corrected some order of operations problems in pgpa_walk_recursively(). The previous coding code deliver incorrect results when an elided SubqueryScan should have been considered the only RTI-bearing plan node beneath a Gather or Gather Merge. - I have adjusted pgpa_planner_apply_joinrel_advice() to fix a problem where GATHER() or GATHER_MERGE() advice could be falsely reported as conflicting with PARTITIONWISE() advice. - I have adjusted pgpa_plan_walker() and pgpa_walk_recursively() to fix a problem with Gather or Gather Merge nodes together with partitionwise aggregation, where the generated plan advice was sometimes incorrect. - I have done some further work on the documentation. Specifically, I added a list of known limitations, or at least the start of one; I expanded the discussion of what "matched" and "partially matched" mean; I expanded the discussion of how NO_GATHER works; and I and changed some references from "table" to "relation". - Comment improvements. As you might or might not guess, a lot of these bug fixes and documentation updates were indirect consequences of the replacement of get_relation_info_hook with buld_simple_rel_hook. Once I made that change, it fixed some problems, but revealed others which I then had to fix or document. However, some of these fixes are just cleaning up other things revealed by running test_plan_advice with pg_plan_advice.feedback_warnings=on. Even though I have been running test_plan_advice locally with pg_plan_advice.feedback_warnings=on, I have not enabled that in this version of the patch set. That's because that still causes 1 test failure, due to self-join elimination doing something that perhaps it shouldn't. More about that later, after I've further analyzed it. -- Robert Haas EDB: http://www.enterprisedb.com
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
pg_plan_advice: Fix another unique-semijoin bug.
- 4321dcad475b 19 (unreleased) landed
-
pg_plan_advice: Export feedback-related definitions.
- c644aca24089 19 (unreleased) landed
-
pg_plan_advice: Fix a bug when a subquery is pruned away entirely.
- 0f93ebb3112d 19 (unreleased) landed
-
pg_plan_advice: Add alternatives test to Makefile.
- 1faf9dfa4796 19 (unreleased) landed
-
pg_plan_advice: Handle non-repeatable TABLESAMPLE scans.
- 3311ccc3d24b 19 (unreleased) landed
-
pg_stash_advice: Allow stashed advice to be persisted to disk.
- c10edb102ada 19 (unreleased) landed
-
Add pg_stash_advice contrib module.
- e8ec19aa321a 19 (unreleased) landed
-
pg_plan_advice: Avoid assertion failure with partitionwise aggregate.
- e2ee95233cab 19 (unreleased) landed
-
pg_plan_advice: Invent DO_NOT_SCAN(relation_identifier).
- 6455e55b0da4 19 (unreleased) landed
-
Add an alternative_plan_name field to PlannerInfo.
- 26255a320733 19 (unreleased) landed
-
pg_plan_advice: Refactor to invent pgpa_planner_info
- 5dcb15e89af2 19 (unreleased) landed
-
Respect disabled_nodes in fix_alternative_subplan.
- 47c110f77e75 19 (unreleased) landed
-
get_memoize_path: Don't exit quickly when PGS_NESTLOOP_PLAIN is unset.
- dc47beacaa0b 19 (unreleased) landed
-
test_plan_advice: Set TAP test priority 50 in meson.build.
- 12444183e401 19 (unreleased) landed
-
pg_plan_advice: Avoid a crash under GEQO.
- 01b02c0ecad1 19 (unreleased) landed
-
Test pg_plan_advice using a new test_plan_advice module.
- e0e4c132ef2b 19 (unreleased) landed
-
pg_plan_advice: Always install pg_plan_advice.h, and in the right place
- 59dcc19b397f 19 (unreleased) landed
-
pg_plan_advice: Fix failures to accept identifier keywords.
- 5e72ce2467c1 19 (unreleased) landed
-
Add pg_plan_advice contrib module.
- 5883ff30b02c 19 (unreleased) landed
-
Allow extensions to mark an individual index as disabled.
- 0fbfd37cefb7 19 (unreleased) landed
-
Replace get_relation_info_hook with build_simple_rel_hook.
- 91f33a2ae92a 19 (unreleased) landed
-
Store information about Append node consolidation in the final plan.
- 7358abcc6076 19 (unreleased) landed
-
Store information about elided nodes in the final plan.
- 0d4391b265f8 19 (unreleased) landed
-
Store information about range-table flattening in the final plan.
- adbad833f3d9 19 (unreleased) landed
-
Pass cursorOptions to planner_setup_hook.
- 0f4c8d33d49d 19 (unreleased) landed
-
Fix PGS_CONSIDER_NONPARTIAL interaction with Materialize nodes.
- cbdf93d47122 19 (unreleased) landed
-
Fix mistakes in commit 4020b370f214315b8c10430301898ac21658143f
- 71c1136989b3 19 (unreleased) landed
-
Allow for plugin control over path generation strategies.
- 4020b370f214 19 (unreleased) landed
-
Update some comments for fasthash
- 7892e2592471 19 (unreleased) landed
-
Allow passing a pointer to GetNamedDSMSegment()'s init callback.
- 48d4a1423d2e 19 (unreleased) cited
-
Don't reset the pathlist of partitioned joinrels.
- 014f9a831a32 19 (unreleased) cited
-
Treat number of disabled nodes in a path as a separate cost metric.
- e22253467942 18.0 cited