Re: RFC: extensible planner state

On Fri, Sep 12, 2025 at 6:34 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> You're missing a word above -- like "modules to store their own"
> This path does not reflect where you put the file

Thanks.

> Storing a uint32 in a signed int that could be 32-bit stuck out to me.

"git grep pg_nextpower2_32" finds examples of assigning the result to
both "int" and "uint32", and I see no practical risk here.

> Since you don't copy the extension name, it might be worth mentioning
> that the caller should provide a literal or at least something that
> will be around later.

Maybe, but there's no obvious reason for any caller to use anything
other than a string literal.

> You used a different variable name here than in the implementation for
> the RelOptInfo parameter.

Oops.

> > 0002 removes join_search_private, as before. Whether it makes sense to
> > go ahead with this is debatable. Needs review, and needs an opinion on
> > whether this should be considered a PoC only (and discarded) or
> > something that should go forward to commit.
>
> Is there a downside to going forward with it?

I think it's just a stylistic preference, whether people like it this
way better or not.

> As for the code itself, I thought assumeReplanning was a bit vague
> since it seems like whether or not replanning is allowed could come up
> outside of join order search -- but perhaps that's okay.

Yeah, there is room for bikeshedding that name.

> > For another example of how these patches could be used, see
> > http://postgr.es/m/CA+TgmoZ=6jJi9TGyZCm33vads46HFkyz6Aju_saLT6GFS-iFug@mail.gmail.com
> > and in particular 0001 and 0002. This patch set's planner_setup_hook
> > call would go write after those patches compute default_ssa_mask and
> > default_jsa_mask, allowing the hook to override those values.
>
> So, are you saying that you would rewrite the patches in that set to
> use the infrastructure in this set -- e.g. remove that set's
> PlannerGlobal.default_jsa_mask and instead put it in
> PlannerGlobal.extension_state? Or am I misunderstanding?

No, that's not what I'm saying. What I'm saying is that with both
patches applied, planner_setup_hook() from this patch ends up getting
called right after default_jsa_mask is set, so another thing this hook
can do is adjust that value. Or, for example, you can write a patch
that uses this infrastructure to associate state with each RelOptInfo,
and then you can use that state to decide how to set jsa_mask in
join_path_setup_hook. In other words, it's easier to make effective
use of those patches if you have the infrastructure provided by these
patches.

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