From: Bruce Momjian <bruce@momjian.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@lists.postgresql.org, Daniel Gustafsson <daniel@yesql.se>
Subject: Re: format of pg_upgrade loadable_libraries warning
Date: Fri, 4 Oct 2019 17:37:46 -0400
Message-ID: <20191004213746.GD29227@momjian.us> (raw)
In-Reply-To: <20191002172337.GC9680@telsasoft.com>
References: <20191002172337.GC9680@telsasoft.com>
On Wed, Oct 2, 2019 at 12:23:37PM -0500, Justin Pryzby wrote:
> 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=60e3bd1d7f92430b24b710ecf0559656e...
>
> 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 :(
Understood. This is a general problem with the way pg_upgrade displays
errors and the databases/objects associated with them. The attached
patch fixes the output text to say "in database", e.g.:
Could not load library "$libdir/pgfincore": ERROR: could not access file "$libdir/pgfincore": No such file or directory
in database: postgres
in database: too
Would intenting help too? I am inclined to fix this only head, and not
to backpatch the change.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
Attachments:
[text/x-diff] error-found.diff (3.7K, ../20191004213746.GD29227@momjian.us/2-error-found.diff)
download | inline diff:diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.cnew file mode 100644index 617270f..1bbb57c
*** a/src/bin/pg_upgrade/check.c
--- b/src/bin/pg_upgrade/check.c
*************** check_for_isn_and_int8_passing_mismatch(
*** 858,864 ****
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s\n",
--- 858,864 ----
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s\n",
*************** check_for_tables_with_oids(ClusterInfo *
*** 937,943 ****
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s\n",
--- 937,943 ----
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s\n",
*************** check_for_reg_data_type_usage(ClusterInf
*** 1046,1052 ****
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
--- 1046,1052 ----
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
*************** check_for_jsonb_9_4_usage(ClusterInfo *c
*** 1137,1143 ****
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
--- 1137,1143 ----
output_path, strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.cnew file mode 100644index 0c66d1c..506fd34
*** a/src/bin/pg_upgrade/function.c
--- b/src/bin/pg_upgrade/function.c
*************** check_loadable_libraries(void)
*** 256,262 ****
}
if (was_load_failure)
! fprintf(script, _("Database: %s\n"),
old_cluster.dbarr.dbs[os_info.libraries[libnum].dbnum].db_name);
}
--- 256,262 ----
}
if (was_load_failure)
! fprintf(script, _("in database: %s\n"),
old_cluster.dbarr.dbs[os_info.libraries[libnum].dbnum].db_name);
}
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.cnew file mode 100644index 10cb362..fba7c6c
*** a/src/bin/pg_upgrade/version.c
--- b/src/bin/pg_upgrade/version.c
*************** old_9_3_check_for_line_data_type_usage(C
*** 157,163 ****
strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
--- 157,163 ----
strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
*************** old_9_6_check_for_unknown_data_type_usag
*** 258,264 ****
strerror(errno));
if (!db_used)
{
! fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
--- 258,264 ----
strerror(errno));
if (!db_used)
{
! fprintf(script, "in database: %s\n", active_db->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us, pryzby@telsasoft.com, daniel@yesql.se
Subject: Re: format of pg_upgrade loadable_libraries warning
In-Reply-To: <20191004213746.GD29227@momjian.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox