public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Fix for pageinspect bug in PG 17
Date: Mon, 11 Nov 2024 19:32:10 +0100
Message-ID: <[email protected]> (raw)
Hi,
Here's a fix for pageinspect bug in PG17, reported in [1]. The bug turns
out to be introduced by my commit
commit dae761a87edae444d11a411f711f1d679bed5941
Author: Tomas Vondra <[email protected]>
Date: Fri Dec 8 17:07:30 2023 +0100
Add empty BRIN ranges during CREATE INDEX
...
This adds an out argument to brin_page_items, but I failed to consider
the user may still run with an older version of the extension - either
after pg_upgrade (as in the report), or when the CREATE EXTENSION
command specifies VERSION.
The new "empty" field is added in the middle of the output tuple, which
shifts the values, causing segfault when accessing a text field at the
end of the array.
Of course, we add arguments to existing functions pretty often, and we
know how to do that in backwards-compatible way - pg_stat_statements is
a good example of how to do that nicely. Unfortunately, it's too late to
do that for brin_page_items :-( There may already be upgraded systems or
with installed pageinspect, etc.
The only fix I can think of is explicitly looking at TupleDesc->natts,
as in the attached fix.
regards
--
Tomas Vondra
Attachments:
[text/x-patch] pageinspect-fix.patch (1.9K, ../[email protected]/2-pageinspect-fix.patch)
download | inline diff:
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 22621d584fa..6086b02445b 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -240,25 +240,29 @@ brin_page_items(PG_FUNCTION_ARGS)
else
{
int att = attno - 1;
+ int idx = 0;
- values[0] = UInt16GetDatum(offset);
+ values[idx++] = UInt16GetDatum(offset);
switch (TupleDescAttr(rsinfo->setDesc, 1)->atttypid)
{
case INT8OID:
- values[1] = Int64GetDatum((int64) dtup->bt_blkno);
+ values[idx++] = Int64GetDatum((int64) dtup->bt_blkno);
break;
case INT4OID:
/* support for old extension version */
- values[1] = UInt32GetDatum(dtup->bt_blkno);
+ values[idx++] = UInt32GetDatum(dtup->bt_blkno);
break;
default:
elog(ERROR, "incorrect output types");
}
- values[2] = UInt16GetDatum(attno);
- values[3] = BoolGetDatum(dtup->bt_columns[att].bv_allnulls);
- values[4] = BoolGetDatum(dtup->bt_columns[att].bv_hasnulls);
- values[5] = BoolGetDatum(dtup->bt_placeholder);
- values[6] = BoolGetDatum(dtup->bt_empty_range);
+ values[idx++] = UInt16GetDatum(attno);
+ values[idx++] = BoolGetDatum(dtup->bt_columns[att].bv_allnulls);
+ values[idx++] = BoolGetDatum(dtup->bt_columns[att].bv_hasnulls);
+ values[idx++] = BoolGetDatum(dtup->bt_placeholder);
+
+ if (rsinfo->setDesc->natts >= 8)
+ values[idx++] = BoolGetDatum(dtup->bt_empty_range);
+
if (!dtup->bt_columns[att].bv_allnulls)
{
BrinValues *bvalues = &dtup->bt_columns[att];
@@ -284,13 +288,15 @@ brin_page_items(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&s, '}');
- values[7] = CStringGetTextDatum(s.data);
+ values[idx++] = CStringGetTextDatum(s.data);
pfree(s.data);
}
else
{
- nulls[7] = true;
+ nulls[idx++] = true;
}
+
+ Assert(idx == rsinfo->setDesc->natts);
}
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
view thread (2+ messages)
reply
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: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Fix for pageinspect bug in PG 17
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox