Date: Mon, 7 Oct 2019 13:47:57 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	pgsql-hackers@lists.postgresql.org,
	Daniel Gustafsson <daniel@yesql.se>
Subject: Re: format of pg_upgrade loadable_libraries warning
Message-ID: <20191007174757.GE4732@momjian.us>
References: <20191002172337.GC9680@telsasoft.com>
 <20191004213746.GD29227@momjian.us>
 <20191004224008.GQ11241@telsasoft.com>
 <20191005001524.GB21896@momjian.us>
 <31405.1570247721@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="BXVAT5kNtrzKuDFl"
Content-Disposition: inline
In-Reply-To: <31405.1570247721@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--BXVAT5kNtrzKuDFl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Oct  4, 2019 at 11:55:21PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Fri, Oct  4, 2019 at 05:40:08PM -0500, Justin Pryzby wrote:
> >> I would argue to include in 12.1, since 12 is what most everyone will use for
> >> upgrades, and patch for .1 will help people upgrading for 11 of the next 12
> >> months.  (But, your patch is more general than mine).
> 
> > No, there might be tools that depend on the existing format, and this is
> > the first report of confusion I have read.
> 
> Translations will also lag behind any such change.  Speaking of which,
> it might be a good idea to include some translator: annotations to
> help translators understand what context these fragmentary phrases
> are used in.  I'd actually say that my biggest concern with these
> messages is whether they can translate into something sane.

Uh, I looked at the pg_ugprade code and the error message is:

        pg_fatal("Your installation contains \"contrib/isn\" functions which rely on the\n"
                 "bigint data type.  Your old and new clusters pass bigint values\n"
                 "differently so this cluster cannot currently be upgraded.  You can\n"
                 "manually upgrade databases that use \"contrib/isn\" facilities and remove\n"
                 "\"contrib/isn\" from the old cluster and restart the upgrade.  A list of\n"
                 "the problem functions is in the file:\n"
                 "    %s\n\n", output_path);

and the "in database" (which I have changed to capitalized "In database"
in the attached patch), looks like:

               fprintf(script, "In database: %s\n", active_db->db_name);

meaning it _isn't_ an output error message, but rather something that
appears in an error file.  I don't think either of these are translated.
Is that wrong?

-- 
  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 +

--BXVAT5kNtrzKuDFl
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="error-found.diff"

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
new file mode 100644
index 617270f..e7bf48a
*** 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.c
new file mode 100644
index 0c66d1c..3cbaab6
*** 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.c
new file mode 100644
index 10cb362..42f1ce7
*** 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",

--BXVAT5kNtrzKuDFl--