On Fri, Aug 18, 2023 at 12:47 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Aug 17, 2023 at 2:10 PM Peter Smith <smithpb2...@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