Feedback-ID: ia2694551:Fastmail
Date: Wed, 20 Sep 2023 18:54:24 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: pg_upgrade --check fails to warn about abstime
Message-ID: <202309201654.ng4ksea25mti@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="g7jrvb2nzjsunkch"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/202309201654.ng4ksea25mti%40alvherre.pgsql>
Precedence: bulk


--g7jrvb2nzjsunkch
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I got a complaint that pg_upgrade --check fails to raise red flags when
the source database contains type abstime when upgrading from pg11.  The
type (along with reltime and tinterval) was removed by pg12.


In passing, while testing this, I noticed that the translation
infrastructure in pg_upgrade/util.c is broken: we do have the messages
in the translation catalog, but the translations for the messages from
prep_status are never displayed.  So I made the quick hack of adding _()
around the fmt, and this was the result:

Verificando Consistencia en Vivo en el Servidor Antiguo
-------------------------------------------------------
Verificando las versiones de los clústers                    éxito
Verificando que el usuario de base de datos es el usuario de instalaciónéxito
Verificando los parámetros de conexión de bases de datos    éxito
Verificando transacciones preparadas                          éxito
Verificando tipos compuestos definidos por el sistema en tablas de usuarioéxito
Verificando tipos de datos reg* en datos de usuario           éxito
Verificando contrib/isn con discordancia en mecanismo de paso de bigintéxito
Checking for incompatible "aclitem" data type in user tables  éxito
Checking for removed "abstime" data type in user tables       éxito
Checking for removed "reltime" data type in user tables       éxito
Checking for removed "tinterval" data type in user tables     éxito
Verificando conversiones de codificación definidas por el usuarioéxito
Verificando operadores postfix definidos por el usuario       éxito
Verificando funciones polimórficas incompatibles             éxito
Verificando tablas WITH OIDS                                  éxito
Verificando columnas de usuario del tipo «sql_identifier»   éxito
Verificando la presencia de las bibliotecas requeridas        éxito
Verificando que el usuario de base de datos es el usuario de instalaciónéxito
Verificando transacciones preparadas                          éxito
Verificando los directorios de tablespaces para el nuevo clústeréxito

Note how nicely they line up ... not.  There is some code that claims to
do this correctly, but apparently it counts bytes, not characters, and
also it appears to be measuring the original rather than the
translation.

I think we're trimming the strings in the wrong places.  We need to
apply _() to the originals, not the trimmed ones.  Anyway, clearly
nobody has looked at this very much.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"We’ve narrowed the problem down to the customer’s pants being in a situation
 of vigorous combustion" (Robert Haas, Postgres expert extraordinaire)

--g7jrvb2nzjsunkch
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-pg_upgrade-check-for-types-removed-in-pg12.patch"

From 0c11fb69019e0c694249eaa70eb325a015f107f1 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 19 Sep 2023 12:30:11 +0200
Subject: [PATCH] pg_upgrade: check for types removed in pg12

Commit cda6a8d01d39 removed a few datatypes, but didn't update
pg_upgrade --check to throw error if these types are used.  So the users
find that pg_upgrade --check tells them that everything is fine, only to
fail when the real upgrade is attempted.
---
 src/bin/pg_upgrade/check.c | 48 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 56e313f562..3a72c5bfd7 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -26,6 +26,8 @@ static void check_for_tables_with_oids(ClusterInfo *cluster);
 static void check_for_composite_data_type_usage(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static void check_for_aclitem_data_type_usage(ClusterInfo *cluster);
+static void check_for_removed_data_type_usage(ClusterInfo *cluster,
+											  const char *datatype);
 static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
 static void check_for_pg_role_prefix(ClusterInfo *cluster);
 static void check_for_new_tablespace_dir(void);
@@ -111,6 +113,16 @@ check_and_dump_old_cluster(bool live_check)
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1500)
 		check_for_aclitem_data_type_usage(&old_cluster);
 
+	/*
+	 * PG 12 removed types abstime, reltime, tinterval.
+	 */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1100)
+	{
+		check_for_removed_data_type_usage(&old_cluster, "abstime");
+		check_for_removed_data_type_usage(&old_cluster, "reltime");
+		check_for_removed_data_type_usage(&old_cluster, "tinterval");
+	}
+
 	/*
 	 * PG 14 changed the function signature of encoding conversion functions.
 	 * Conversions from older versions cannot be upgraded automatically
@@ -1215,7 +1227,8 @@ check_for_aclitem_data_type_usage(ClusterInfo *cluster)
 {
 	char		output_path[MAXPGPATH];
 
-	prep_status("Checking for incompatible \"aclitem\" data type in user tables");
+	prep_status("Checking for incompatible \"%s\" data type in user tables",
+				"aclitem");
 
 	snprintf(output_path, sizeof(output_path), "tables_using_aclitem.txt");
 
@@ -1233,6 +1246,39 @@ check_for_aclitem_data_type_usage(ClusterInfo *cluster)
 		check_ok();
 }
 
+/*
+ * check_for_removed_data_type_usage
+ *
+ *	similar to the above, but for types that were removed in 12.
+ */
+static void
+check_for_removed_data_type_usage(ClusterInfo *cluster, const char *datatype)
+{
+	char		output_path[MAXPGPATH];
+	char		typename[NAMEDATALEN];
+
+	prep_status("Checking for removed \"%s\" data type in user tables",
+				datatype);
+
+	snprintf(output_path, sizeof(output_path), "tables_using_%s.txt",
+			 datatype);
+	snprintf(typename, sizeof(typename), "pg_catalog.%s", datatype);
+
+	if (check_for_data_type_usage(cluster, typename, output_path))
+	{
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("Your installation contains the \"%s\" data type in user tables.\n"
+				 "Data type \"%s\" has been removed in PostgreSQL version 12,\n"
+				 "so this cluster cannot currently be upgraded.  You can drop the\n"
+				 "problem columns, or change them to another data type, and restart\n"
+				 "the upgrade.  A list of the problem columns is in the file:\n"
+				 "    %s", datatype, datatype, output_path);
+	}
+	else
+		check_ok();
+}
+
+
 /*
  * check_for_jsonb_9_4_usage()
  *
-- 
2.39.2


--g7jrvb2nzjsunkch--