Re: RFC: extensible planner state

On 25/8/2025 21:46, Robert Haas wrote:
> On Wed, Aug 20, 2025 at 3:13 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> Here's v2. 0001 is what you saw before with an attempt to fix the
>> memory context handling. 0002 removes join_search_private. All I've
>> tested is that the tests pass.
> 
> Here's v3 with a few more patches. I'm now fairly confident I have the
> basic approach correct here, but let's see what others think.
> 
> 0001 is the core "private state" patch for PlannerGlobal, PlannerInfo,
> and RelOptInfo. It is unchanged since v2, and contains only the fix
> for memory context handling since v1. However, I've now tested it, and
> I think it's OK to commit, barring further review comments.
Reading this patch, I didn't find reasoning for the two decisions:
1. Why is it necessary to maintain the GetExplainExtensionId and 
GetPlannerExtensionId routines? It seems that using a single 
extension_id (related to the order of the library inside the 
file_scanner) is more transparent and more straightforward if necessary.
2. Why does the extensibility approach in 0001 differ from that in 0004? 
I can imagine it is all about limiting extensions, but anyway, a module 
has access to PlannerInfo, PlannerGlobal, etc. So, this machinery looks 
a little redundant, doesn't it?
> 0003 adds two new planner hooks. In experimenting with 0001, I
> discovered that it was a little hard to use. PlannerGlobal has to do
> with what happens in a whole planning cycle, but the only hook we have
> that's in approximately the right place is planner_hook, and it can't
> see the PlannerGlobal object. So, I added these hooks. The first fires
> after PlannerGlobal is fully initialized and before we start using it,
> and the second fires just before we throw PlannerGlobal away. I
> considered some other approaches, specifically: (1) making
> subquery_planner a hook, (2) making grouping_planner a hook, and (3)
> doing as the patch does but with the call before rather than after
> assembling the PlannedStmt. Those proved inferior; the hook at the
> very end of planner() just before we discard the PlannerGlobal object
> appears quite valuable to me. Needs review.These hooks look contradictory to me. If we store data inside a 
RelOptInfo, it will be challenging to match this RelOptInfo with 
specific Plan node(s) in the shutdown hook. That's why I prefer to use 
create_plan_hook, which may also utilise PlannerGlobal and store the 
extension's data within the plan.

I support the subquery_planner hook idea because each subplan represents 
a separate planning space, and it can be challenging to distinguish 
between two similar subplans that exist at the same query level.

--
Andrei Lepikhov

-- 
regards, Andrei Lepikhov