Hi Jakub, On Tue, Apr 01, 2025 at 12:56:06PM +0200, Jakub Wartak wrote: > On Mon, Mar 31, 2025 at 4:59 PM Bertrand Drouvot > <bertranddrouvot...@gmail.com> wrote: > > > Hi, > > Hi Bertrand, happy to see you back, thanks for review and here's v18 > attached (an ideal fit for PG18 ;))
Thanks for the new version! === About v18-0002 It looks in a good shape to me. The helper's name might still be debatable though. I just have 2 comments: === 1 + if (bufRecord->blocknum == InvalidBlockNumber || + bufRecord->isvalid == false) It seems to me that this check could now fit in one line. === 2 + { SRF_RETURN_DONE(funcctx); + } Extra parentheses are not needed. === About v18-0003 === 3 I think that pg_buffercache--1.5--1.6.sql is not correct. It should contain only the necessary changes when updating from 1.5. It means that it should "only" create the new objects (views and functions in our case) that come in v18-0003 and grant appropriate privs. Also it should mention: " \echo Use "ALTER EXTENSION pg_buffercache UPDATE TO '1.6'" to load this file. \quit " and not: " +\echo Use "CREATE EXTENSION pg_buffercache" to load this file. \quit " The already existing pg_buffercache--1.N--1.(N+1).sql are good examples. === 4 + for (i = 0; i < NBuffers; i++) + { + int blk2page = (int) i * pages_per_blk; + I think that should be: int blk2page = (int) (i * pages_per_blk); === About v18-0004 === 5 When running: select c.name, c.size as num_size, s.size as shmem_size from (select n.name as name, sum(n.size) as size from pg_shmem_numa_allocations n group by n.name) c, pg_shmem_allocations s where c.name = s.name; I can see: - pg_shmem_numa_allocations reporting a lot of times the same size - pg_shmem_numa_allocations and pg_shmem_allocations not reporting the same size Do you observe the same? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com