format of pg_upgrade loadable_libraries warning

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: format of pg_upgrade loadable_libraries warning
Date: 2019-10-02 17:23:37
Message-ID: 20191002172337.GC9680@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Regarding the previous thread and commit here:
https://www.postgresql.org/message-id/flat/20180713162815.GA3835%40momjian.us
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=60e3bd1d7f92430b24b710ecf0559656eb8ed499

I'm suggesting to reformat the warning, which I found to be misleading:

|could not load library "$libdir/pgfincore": ERROR: could not access file "$libdir/pgfincore": No such file or directory
|Database: postgres
|Database: too

To me that reads as "error message" followed by successful processing of two,
named database, and not "error message followed by list of databases for which
that error was experienced". Essentially, the database names are themselves
the "error", and the message is a prefix indicating the library version; but
normally, error-looking things are output without a "prefix", since they
weren't anticipated.

The existing check is optimized to check each library once, but then outputs
each database which would try to load it. That's an implementation detail, but
adds to confusion, since it shows a single error-looking thing which might
apply to multiple DBs (not obvious to me that it's associated with an DB at
all). That leads me to believe that after I "DROP EXTENSION" once, I can
reasonably expect the upgrade to complete, which has a good chance of being
wrong, and is exactly what the patch was intended to avoid :(

To reproduce:

$ /usr/pgsql-11/bin/initdb -D ./pgtestlib11
$ /usr/pgsql-12/bin/initdb -D ./pgtestlib12
$ /usr/pgsql-11/bin/pg_ctl -D ./pgtestlib11 -o '-c port=5678 -c unix_socket_directories=/tmp' start
$ psql postgres -h /tmp -p 5678 -c 'CREATE EXTENSION pgfincore' -c 'CREATE DATABASE too'
$ psql too -h /tmp -p 5678 -c 'CREATE EXTENSION pgfincore'
$ /usr/pgsql-11/bin/pg_ctl -D ./pgtestlib11 stop
$ /usr/pgsql-12/bin/pg_upgrade -b /usr/pgsql-11/bin -B /usr/pgsql-12/bin -d ./pgtestlib11 -D pgtestlib12
$ cat loadable_libraries.txt
Could not load library "$libdir/pgfincore": ERROR: could not access file "$libdir/pgfincore": No such file or directory
Database: postgres
Database: too

I concede that the situation is clearer if there are multiple libraries causing
errors, especially in overlapping list of databases:

|[pryzbyj(at)database ~]$ cat loadable_libraries.txt
|could not load library "$libdir/pg_repack": ERROR: could not access file "$libdir/pg_repack": No such file or directory
|Database: postgres
|Database: too
|could not load library "$libdir/pgfincore": ERROR: could not access file "$libdir/pgfincore": No such file or directory
|Database: postgres
|Database: too

I think the list of databases should be formatted to indicate its association
with the preceding error by indentation and verbage, or larger refactoring to
present in a list, like:
"Databases with library which failed to load: %s: %s",
PQerrorMessage(conn), list_of_dbs_loading_that_lib

Justin

Attachment Content-Type Size
v1-0001-Indent-and-repeat-name-of-library-failing-to-load.patch text/x-diff 1020 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-10-02 17:23:54 Re: Hooks for session start and end, take two
Previous Message Tom Lane 2019-10-02 17:21:51 Re: Possible bug: SQL function parameter in window frame definition