On Thu, Mar 12, 2026 at 7:46 PM Alexandra Wang
<alexandra.wang.oss@gmail.com> wrote:
> For the make tests, I had to add
>
> +EXTRA_INSTALL = contrib/pg_plan_advice
Thanks, will fix.
> There is a typo in the commit message:
> "pg_stash.advice_stash" should be "pg_stash_advice.stash_name".
Thanks, will fix.
> I doubt there will be more than 2 billion stashes in the system, but
> if in any case we reach that number, we don't handle int overflow.
> Should we set a limit on how many stashes can be stored?
I think the thing to do here is just change the stash ID to a 64-bit
value. I'll do that in the next version. We assume in numerous places
that a 64-bit integer never overflows (e.g. LSNs) which is pretty fair
considering how long it would take for that to happen.
> Nit:
> find_defelem_by_defname() is defined in all three modules, and also in
> pg_plan_advice. Knowing it is very small, would it make sense to
> extern the one in pg_plan_advice and reuse it?
I think if we want to avoid duplicating this function, we should
actually put it someplace in core, e.g. expose it via defrem.h and put
the code in commands/define.c. Any user of extendplan.h is likely to
need a function like this, but we shouldn't put it there because there
could be other use cases as well.
--
Robert Haas
EDB: http://www.enterprisedb.com
