Re: [PoC] pg_upgrade: allow to upgrade publisher node

On Fri, Aug 18, 2023 at 12:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Aug 17, 2023 at 2:10 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Here are some review comments for the first 2 patches.
> >
> >   /*
> > - * Fetch all libraries containing non-built-in C functions in this DB.
> > + * Fetch all libraries containing non-built-in C functions and
> > + * output plugins in this DB.
> >   */
> >   ress[dbnum] = executeQueryOrDie(conn,
> >   "SELECT DISTINCT probin "
> >   "FROM pg_catalog.pg_proc "
> >   "WHERE prolang = %u AND "
> >   "probin IS NOT NULL AND "
> > - "oid >= %u;",
> > + "oid >= %u "
> > + "UNION "
> > + "SELECT DISTINCT plugin "
> > + "FROM pg_catalog.pg_replication_slots "
> > + "WHERE wal_status <> 'lost' AND "
> > + "database = current_database() AND "
> > + "temporary IS FALSE;",
> >   ClanguageId,
> >   FirstNormalObjectId);
> >   totaltups += PQntuples(ress[dbnum]);
> >
> > ~
> >
> > Maybe it is OK, but it somehow seems like the new logic has been
> > jammed into the get_loadable_libraries() function for coding
> > convenience. For example, all the names (function names, variable
> > names, structure field names) are referring to "libraries", so the
> > plugin seems a bit out of place.
> >
>
> But the same name library (as plugin) should exist for the upgrade of
> slots. I feel doing it separately could either lead to a redundant
> code or a different way to achieve the same thing. Do you envision any
> problem which we are not seeing?
>

No problem. I'd misunderstood that the "plugin" referred to here is a
shared object file (aka library) name, so it does belong here after
all. I think the new comments could be made more clear about this
point though.

------
Kind Regards,
Peter Smith.
Fujitsu Australia