MIME-Version: 1.0
References: <19508-e5f188183279219b@postgresql.org>
 <CAHGQGwGaC8RNeVP3A4_R1OcHJ-DrXgr2HH2UEYrt-Ueu516Gng@mail.gmail.com>
In-Reply-To: 
 <CAHGQGwGaC8RNeVP3A4_R1OcHJ-DrXgr2HH2UEYrt-Ueu516Gng@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Fri, 5 Jun 2026 09:12:20 +0530
Message-ID: 
 <CAJTYsWX6b0UxeZ9HGGnju-XpWRmLr_y8AVeyWqd3gL+a0NkGtw@mail.gmail.com>
Subject: Re: BUG #19508: pg_buffercache_pages() crashes the backend with an
 incompatible caller-supplied record definition
To: Fujii Masao <masao.fujii@gmail.com>
Cc: n.kalinin@postgrespro.ru, pgsql-bugs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="0000000000005d9930065379732c"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWX6b0UxeZ9HGGnju-XpWRmLr_y8AVeyWqd3gL%2Ba0NkGtw%40mail.gmail.com>
Precedence: bulk

--0000000000005d9930065379732c
Content-Type: text/plain; charset="UTF-8"

Hi,

On Fri, 5 Jun 2026 at 08:49, Fujii Masao <masao.fujii@gmail.com> wrote:

>
> Commit 257c8231bf9 changed pg_buffercache_pages() to materialize rows
> directly
> into a tuplestore. As a result, the function started using the
> caller-supplied
> RECORD descriptor as rsinfo->setDesc, so a mismatched column definition
> list
> could cause tuplestore_putvalues() to interpret returned Datums with
> incorrect
> types.
>
> Before that change, pg_buffercache_pages() exposed its actual tuple
> descriptor
> to the executor, allowing the executor's existing rowtype checks to reject
> incompatible definitions with a normal error.
>
> The attached patch restores that behavior while keeping the
> materialized-SRF
> implementation. Thoughts?
>

Thanks for the patch, Fujii-san!

I was looking into the bug last night, and the approach looks right to me.

This still means InitMaterializedSRF() briefly creates the caller-derived
descriptor before rsinfo->setDesc is replaced. That seems acceptable here:
the descriptor lives only in the per-query context, and avoiding a local
copy of InitMaterializedSRF() keeps the fix much smaller and less fragile.

One small nit: build_buffercache_pages_tupledesc() names attribute 8
"usage_count", while the existing pg_buffercache view and the test use
"usagecount". This probably does not affect the tupledesc_match() check,
but I think it would be better to keep the existing spelling for
consistency.

Regards,
Ayush

--0000000000005d9930065379732c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Fri, 5 Jun 20=
26 at 08:49, Fujii Masao &lt;<a href=3D"mailto:masao.fujii@gmail.com">masao=
.fujii@gmail.com</a>&gt; wrote:</div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex">
<br>
Commit 257c8231bf9 changed pg_buffercache_pages() to materialize rows direc=
tly<br>
into a tuplestore. As a result, the function started using the caller-suppl=
ied<br>
RECORD descriptor as rsinfo-&gt;setDesc, so a mismatched column definition =
list<br>
could cause tuplestore_putvalues() to interpret returned Datums with incorr=
ect<br>
types.<br>
<br>
Before that change, pg_buffercache_pages() exposed its actual tuple descrip=
tor<br>
to the executor, allowing the executor&#39;s existing rowtype checks to rej=
ect<br>
incompatible definitions with a normal error.<br>
<br>
The attached patch restores that behavior while keeping the materialized-SR=
F<br>
implementation. Thoughts?<br></blockquote><div><br></div><div>Thanks for th=
e patch, Fujii-san!<br><br>I was looking into the bug last night, and the a=
pproach looks right to me.</div><div><br></div><div>This still means InitMa=
terializedSRF() briefly creates the caller-derived<br>descriptor before rsi=
nfo-&gt;setDesc is replaced. That seems acceptable here:<br>the descriptor =
lives only in the per-query context, and avoiding a local<br>copy of InitMa=
terializedSRF() keeps the fix much smaller and less fragile.<br><br>One sma=
ll nit: build_buffercache_pages_tupledesc() names attribute 8<br>&quot;usag=
e_count&quot;, while the existing pg_buffercache view and the test use<br>&=
quot;usagecount&quot;. This probably does not affect the tupledesc_match() =
check,<br>but I think it would be better to keep the existing spelling for<=
br>consistency.</div><div><br></div><div>Regards,<br>Ayush</div></div></div=
>

--0000000000005d9930065379732c--