Re: Add os_page_num to pg_buffercache

Hi,

Thanks for the prompt reply!  

Yeah, I tried to avoid code duplication for the "os pages" related stuff in
v1. I can check if more can be done (outside of the "os pages" related stuff).

Might be done in a dedicated patch though, I mean I don't think that should be
a blocker for this one.

Agreed, if there's any low-hanging fruit to address now that this file is cracked open, then great. Otherwise, makes sense to leave it for a separate dedicated patch. 

If you don't mind I have some further questions on the patch, see below.

+		if (get_call_result_type(fcinfo, NULL, &expected_tupledesc) != TYPEFUNC_COMPOSITE)
+			elog(ERROR, "return type must be a row type");

Is this needed in the new pg_buffercache_os_pages function? I noticed this check also in the "original" pg_buffercache_pages. There's a comment there indicating that (if I understand correctly) its purpose is to handle upgrades from version 1.0, mentioning a field unrelated to this patch. 

If it's needed, shall we consider adding a similar comment as in pg_buffercache_pages?

+		/*
+		 * Different database block sizes (4kB, 8kB, ..., 32kB) can be used,
+		 * while the OS may have different memory page sizes.
+		 *
+		 * To correctly map between them, we need to: 1. Determine the OS
+		 * memory page size 2. Calculate how many OS pages are used by all
+		 * buffer blocks 3. Calculate how many OS pages are contained within
+		 * each database block.
+		 */

For step number 3 - should it be the other way around: database blocks are contained within OS pages?

Kind regards,

Mircea Cadariu