postgres.git / summary / log / commit / refs

commit    691e8b2e1889d61df47ae76601fa9db6cbac6f1c
Author:   Peter Geoghegan <pg@bowt.ie>
Date:     Sun Mar 08 00:44:53 2020 +0000
First released in: 13.0

    pageinspect: Fix types used for bt_metap() columns.
    
    The data types that contrib/pageinspect's bt_metap() function were
    declared to return as OUT arguments were wrong in some cases.  For
    example, the oldest_xact column (a TransactionId/xid field) was declared
    integer/int4 within the pageinspect extension's sql file.  This led to
    errors when an oldest_xact value that exceeded 2^31-1 was encountered.
    Some of the other columns were defined incorrectly ever since
    pageinspect was first introduced, though they were far less likely to
    produce problems in practice.
    
    Fix these issues by changing the declaration of bt_metap() to
    consistently use data types that can reliably represent all possible
    values.  This fixes things on HEAD only.  No backpatch, since it doesn't
    seem like there is a safe way to fix the issue without including a new
    version of the pageinspect extension (HEAD/Postgres 13 already
    introduced a new version of the extension).  Besides, the oldest_xact
    issue has been around since the release of Postgres 11, and we haven't
    heard any complaints about it before now.
    
    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.
    It seems better to throw an error in a reliable and obvious way
    following a Postgres upgrade, rather than letting bt_metap() fail
    unpredictably.  The problem is fundamentally with the CREATE FUNCTION
    declared data types themselves, so I see no sensible alternative.
    
    Reported-By: Victor Yegorov
    Bug: #16285
    Discussion: https://postgr.es/m/16285-df8fc1000ab3d5fc@postgresql.org


contrib/pageinspect/btreefuncs.c | 27 +++++++++++++++++++++++---- contrib/pageinspect/pageinspect--1.7--1.8.sql | 12 ++++++------ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index dffb3e199ba..e6a2fc1e155 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,30 @@ 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 incorrectly used int4 for certain columns. This caused + * various problems. For example, an int4 version of the "oldest_xact" + * column would not work with TransactionId values that happened to exceed + * PG_INT32_MAX. + * + * There is no way to reliably avoid the problems created by the old + * function definition at this point, so insist that the user update the + * extension. + */ + 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 e34c214c93c..45031a026a6 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; [parent: b9c3de62cbc9]