Horton

Re: RFC: extensible planner state

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-09-24T16:02:58Z
Lists: pgsql-hackers

Attachments

On Mon, Sep 22, 2025 at 3:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I looked through the v5 patchset.

Thanks!

Here's a new set of patches. I've added a new 0001 at the beginning to
fix the bug you identified in SetExplainExtensionState; this will need
to be back-patched to v18 once the release freeze lifts. I've also
adjusted the previous patch, now 0002, to fix the equivalent problem
with the new Set*ExplainState functions,and I've attempted to fix the
other problems you mentioned, with this exception:

> Also, although you're not the first sinner, I really think
> this new argument to planner() should be documented.  Maybe
> the rest too while we're at it.

I'm a little nervous about this. I fear that the comments are all
going to be of the form "to save a file, click the File menu, then
click Save," which doesn't actually help anyone. That might be a
slight exaggeration, but I feel like it's pretty obvious on visual
inspection that Query *parse is what we're planning, char
*query_string is the text version of that, cursorOptions is some kind
of flag mask, boundParams is the parameter values. It's fair to say
that ExplainState *es is a little less obvious than the others, but
saying "If this function is being invoked by EXPLAIN, then
ExplainState *es is the ExplainState, else it is NULL" doesn't really
seem all that helpful to me. I mean, what else would it be? My thought
here would be that if you want to write some comments that you
consider helpful for the existing arguments, I'll try to write a new
comment for this one in the same style (or you can suggest one) and
hold my nose if I don't find it helpful, or alternatively, we could
proceed with these patches without the comment and you can add
whatever comment text you want after-the-fact. If you want the comment
changes in this patch set, then I need a suggestion as to what that
should look like.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. Add extension_state member to PlannedStmt.

  2. Add planner_setup_hook and planner_shutdown_hook.

  3. Add ExplainState argument to pg_plan_query() and planner().

  4. Remove PlannerInfo's join_search_private method.

  5. Allow private state in certain planner data structures.

  6. Fix array allocation bugs in SetExplainExtensionState.

  7. Make it possible for loadable modules to add EXPLAIN options.