On 25/8/2025 21:46, Robert Haas wrote:
On Wed, Aug 20, 2025 at 3:13 PM Robert Haas <robertmh...@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


Reply via email to