v1-0001-pageinspect-Fix-bt_metap-column-types.patch

From cc09ac109ef4110e1144e6d17435eca9938cfa45 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Thu, 5 Mar 2020 17:31:43 -0800
Subject: [PATCH v1] pageinspect: Fix bt_metap() column types.

The types that contrib/pageinspect's bt_metap() function are declared to
return as OUT parameters have been wrong from the start.  This became
more noticeable when commit 857f9c36 introduced the oldest_xact field, a
TransactionId/xid field that was declared integer/int4.  Root blocks are
almost never located at a block number that exceeds INT32_MAX, which
must have been why the problem was overlooked for many years.  The same
cannot be said for transaction IDs, though.  The problem could lead to
errors like: 'value "2180413846" is out of range for type integer'.

Fix the problem in the obvious way: by changing the declaration of
bt_metap() to use types that are wide enough to work reliably.  Also,
throw an error when we detect a bt_metap() declaration that must be from
an old version of the pageinspect extension by examining the number of
attributes from the tuple descriptor for the return tuples.  This kludge
is also used in pg_stat_statements for roughly the same reason.  We
don't bother trying to make the old declaration work here, though,
because the old declaration is fundamentally wrong.

No backpatch because it doesn't seem like there is a safe way to fix the
issue short of including a new version of the pageinspect extension
(Postgres 13 will need a new version anyway, following commit 93ee38ea,
and other commits).  Also, the worst consequences of the bug are that
the user sees a not-particularly-helpful error.

No documentation changes are required, because the documentation handles
bt_metap() without ever making it clear which exact data types are used.
There should be a compatibility note about this change in the Postgres
13 release notes for the sake of completeness, though.

Reported-By: Victor Yegorov
Bug: #16285
Discussion: https://postgr.es/m/16285-df8fc1000ab3d5fc@postgresql.org
---
 contrib/pageinspect/btreefuncs.c              | 20 +++++++++++++++----
 contrib/pageinspect/pageinspect--1.7--1.8.sql | 12 +++++------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 2ddedef714..34352a45b5 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -597,6 +597,8 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
 	}
 }
 
+/* Number of output arguments (columns) for bt_metap() */
+#define BT_METAP_COLS_V1_8	9
 
 /* ------------------------------------------------
  * bt_metap()
@@ -653,13 +655,23 @@ bt_metap(PG_FUNCTION_ARGS)
 	if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
+	/*
+	 * We need a kluge here to detect API versions prior to 1.8.  Earlier
+	 * versions used the wrong data types for some columns.
+	 */
+	if (tupleDesc->natts < BT_METAP_COLS_V1_8)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("function has wrong number of declared columns"),
+				 errhint("To resolve the problem, update the \"pageinspect\" extension to the latest version.")));
+
 	j = 0;
 	values[j++] = psprintf("%d", metad->btm_magic);
 	values[j++] = psprintf("%d", metad->btm_version);
-	values[j++] = psprintf("%d", metad->btm_root);
-	values[j++] = psprintf("%d", metad->btm_level);
-	values[j++] = psprintf("%d", metad->btm_fastroot);
-	values[j++] = psprintf("%d", metad->btm_fastlevel);
+	values[j++] = psprintf(INT64_FORMAT, (int64) metad->btm_root);
+	values[j++] = psprintf(INT64_FORMAT, (int64) metad->btm_level);
+	values[j++] = psprintf(INT64_FORMAT, (int64) metad->btm_fastroot);
+	values[j++] = psprintf(INT64_FORMAT, (int64) metad->btm_fastlevel);
 
 	/*
 	 * Get values of extended metadata if available, use default values
diff --git a/contrib/pageinspect/pageinspect--1.7--1.8.sql b/contrib/pageinspect/pageinspect--1.7--1.8.sql
index e34c214c93..45031a026a 100644
--- a/contrib/pageinspect/pageinspect--1.7--1.8.sql
+++ b/contrib/pageinspect/pageinspect--1.7--1.8.sql
@@ -22,12 +22,12 @@ DROP FUNCTION bt_metap(text);
 CREATE FUNCTION bt_metap(IN relname text,
     OUT magic int4,
     OUT version int4,
-    OUT root int4,
-    OUT level int4,
-    OUT fastroot int4,
-    OUT fastlevel int4,
-    OUT oldest_xact int4,
-    OUT last_cleanup_num_tuples real,
+    OUT root int8,
+    OUT level int8,
+    OUT fastroot int8,
+    OUT fastlevel int8,
+    OUT oldest_xact xid,
+    OUT last_cleanup_num_tuples float8,
     OUT allequalimage boolean)
 AS 'MODULE_PATHNAME', 'bt_metap'
 LANGUAGE C STRICT PARALLEL SAFE;
-- 
2.17.1