I had a look into this patch and would like to share some of my review
comments that requires author's attention.
1) The comment for page_checksum() needs to be corrected. It seems
like it has been copied from page_header and not edited it further.
* Allows inspection of page header fields of a raw page
2) It seems like you have choosen wrong datatype for page_checksum. I
am getting negative checksum value when trying to run below query. I
think the return type for the SQL function page_checksum should be
'integer' instead of 'smallint'.
postgres=# SELECT page_checksum(get_raw_page('pg_class', 0), 100);
Above problem also exist in case of page_header. I am facing similar
problem with page_header as well.
postgres=# SELECT page_header(get_raw_page('pg_class', 0));
3) Any reason for not marking bt_page_items or page_checksum as PARALLEL_SAFE.
4) Error messages in new bt_page_items are not consistent with old
bt_page_items. For eg. if i pass meta page blocknumber as input to
bt_page_items the error message thrown by new and old bt_page_items
postgres=# SELECT * FROM bt_page_items('btree_index', 0) LIMIT 1;
ERROR: block 0 is a meta page
postgres=# SELECT * FROM bt_page_items(get_raw_page('btree_index', 0)) LIMIT 1;
ERROR: block is a meta page
postgres=# SELECT * FROM bt_page_items(get_raw_page('btree_index',
1024)) LIMIT 1;
ERROR: block number 1024 is out of range for relation "btree_index"
postgres=# SELECT * FROM bt_page_items('btree_index', 1024) LIMIT 1;
ERROR: block number out of range
5) Code duplication in bt_page_items() and bt_page_items_bytea() needs
to be handled.
On Tue, Mar 7, 2017 at 9:39 PM, Peter Eisentraut
> On 3/6/17 16:33, Tomas Vondra wrote:
>>> I think it would be better not to maintain so much duplicate code
>>> between bt_page_items(text, int) and bt_page_items(bytea). How about
>>> just redefining bt_page_items(text, int) as an SQL-language function
>>> calling bt_page_items(get_raw_page($1, $2))?
>> Maybe. We can also probably share the code at the C level, so I'll look
>> into that.
> I think SQL would be easier, but either way some reduction in
> duplication would be good.
>>> For page_checksum(), the checksums are internally unsigned, so maybe
>>> return type int on the SQL level, so that there is no confusion about
>>> the sign?
>> That ship already sailed, I'm afraid. We already have checksum in the
>> page_header() output, and it's defined as smallint there. So using int
>> here would be inconsistent.
> OK, no worries then.
> Peter Eisentraut http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> Sent via pgsql-hackers mailing list (firstname.lastname@example.org)
> To make changes to your subscription:
Sent via pgsql-hackers mailing list (email@example.com)
To make changes to your subscription: