Horton

Thread

Commits

  1. pg_buffercache: Fix incorrect result cast for relforknumber

  1. Type of pg_buffercache_pages()::forknum

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-11-12T05:16:12Z

    Hi All,
The SQL definition of pg_buffercache_pages() and pg_buffercache
declares relforknum as int2

CREATE FUNCTION pg_buffercache_pages()
RETURNS SETOF RECORD
AS 'MODULE_PATHNAME', 'pg_buffercache_pages'
LANGUAGE C PARALLEL SAFE;

-- Create a view for convenient access.
CREATE VIEW pg_buffercache AS
SELECT P.* FROM pg_buffercache_pages() AS P
(bufferid integer, relfilenode oid, reltablespace oid, reldatabase oid,
relforknumber int2, relblocknumber int8, isdirty bool, usagecount int2,
^^^^^^^^^^^^^^^^^
pinning_backends int4);

But C implementation uses ObjectIdGetDatum() where ObjectId is 32 bit integer.
values[4] = ObjectIdGetDatum(fctx->record[i].forknum);

Since forknum values are within 16 bits and that's unlikely to change
in future, int2 seems to ok. Also casting it to a wider integer, OID,
also doesn't seem like a correctness issue. But it does look
inconsistent and a reader of C implementation may think that the SQL
datatype used is int4. Should we instead change the C Implementation
to be

values[4] = Int16GetDatum((int16) fctx->record[i].forknum);

The code has been there for 17 years, and I didn't find any previous
complaints. I propose to change just the master branch.

-- 
Best Wishes,
Ashutosh Bapat

  2. Re: Type of pg_buffercache_pages()::forknum

    Michael Paquier <michael@paquier.xyz> — 2025-11-18T06:29:57Z

    On Wed, Nov 12, 2025 at 10:46:12AM +0530, Ashutosh Bapat wrote:
> But C implementation uses ObjectIdGetDatum() where ObjectId is 32 bit integer.
> values[4] = ObjectIdGetDatum(fctx->record[i].forknum);
> 
> Since forknum values are within 16 bits and that's unlikely to change
> in future, int2 seems to ok. Also casting it to a wider integer, OID,
> also doesn't seem like a correctness issue. But it does look
> inconsistent and a reader of C implementation may think that the SQL
> datatype used is int4. Should we instead change the C Implementation
> to be
> 
> values[4] = Int16GetDatum((int16) fctx->record[i].forknum);
> 
> The code has been there for 17 years, and I didn't find any previous
> complaints. I propose to change just the master branch.

This just looks like a copy-pasto to me.  Nice catch, will fix on
HEAD.
--
Michael