Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-04-01 Thread Ashutosh Sharma
Hi Tomas, On Fri, Mar 31, 2017 at 11:05 PM, Tomas Vondra wrote: > On 03/31/2017 06:01 PM, Ashutosh Sharma wrote: >> >> >> It seems like the latest patch(v4) shared by Tomas upthread is an >> empty patch. If I am not wrong, please share the correct patch. >> Thanks.

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-31 Thread Tomas Vondra
On 03/31/2017 06:01 PM, Ashutosh Sharma wrote: It seems like the latest patch(v4) shared by Tomas upthread is an empty patch. If I am not wrong, please share the correct patch. Thanks. OMG, this is the second time I managed to generate an empty patch. I really need to learn not to do that

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-31 Thread Ashutosh Sharma
Hi, On Wed, Mar 29, 2017 at 8:38 PM, Tomas Vondra wrote: > > > On 03/24/2017 04:27 AM, Peter Eisentraut wrote: >> >> On 3/17/17 18:35, Tomas Vondra wrote: >>> >>> On 03/17/2017 05:23 PM, Peter Eisentraut wrote: I'm struggling to find a good way to share

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-29 Thread Tomas Vondra
On 03/24/2017 04:27 AM, Peter Eisentraut wrote: On 3/17/17 18:35, Tomas Vondra wrote: On 03/17/2017 05:23 PM, Peter Eisentraut wrote: I'm struggling to find a good way to share code between bt_page_items(text, int4) and bt_page_items(bytea). If we do it via the SQL route, as I had

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-23 Thread Peter Eisentraut
On 3/17/17 18:35, Tomas Vondra wrote: > On 03/17/2017 05:23 PM, Peter Eisentraut wrote: >> I'm struggling to find a good way to share code between >> bt_page_items(text, int4) and bt_page_items(bytea). >> >> If we do it via the SQL route, as I had suggested, it makes the >> extension

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-17 Thread Tomas Vondra
Hi, On 03/17/2017 05:23 PM, Peter Eisentraut wrote: I'm struggling to find a good way to share code between bt_page_items(text, int4) and bt_page_items(bytea). If we do it via the SQL route, as I had suggested, it makes the extension non-relocatable, and it will also create a bit of a mess

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-17 Thread Peter Eisentraut
I'm struggling to find a good way to share code between bt_page_items(text, int4) and bt_page_items(bytea). If we do it via the SQL route, as I had suggested, it makes the extension non-relocatable, and it will also create a bit of a mess during upgrades. If doing it in C, it will be a bit

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-17 Thread Peter Eisentraut
I have committed the page_checksum function, will work on the bt stuff next. I left in the superuser check, because I was not confident how well pg_checksum_page() would handle messed up data. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support,

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-14 Thread Ashutosh Sharma
On Mar 14, 2017 5:37 PM, "Alvaro Herrera" wrote: Ashutosh Sharma wrote: > Yes. But, as i said earlier I am getting negative checksum value for > page_header as well. Isn't that wrong. For eg. When I debug the > following query, i could pd_checksum value as '40074' in

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-14 Thread Alvaro Herrera
Ashutosh Sharma wrote: > Yes. But, as i said earlier I am getting negative checksum value for > page_header as well. Isn't that wrong. For eg. When I debug the > following query, i could pd_checksum value as '40074' in gdb where > page_header shows it as '-25462'. Yes; the point is that this is

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-13 Thread Ashutosh Sharma
Hi, > >> 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

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-13 Thread Tomas Vondra
On 03/13/2017 06:49 AM, Ashutosh Sharma wrote: Hi, 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

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-12 Thread Ashutosh Sharma
Hi, 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. /* * page_header * * Allows inspection

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-07 Thread 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))?

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-06 Thread Tomas Vondra
On 03/06/2017 10:13 PM, Peter Eisentraut wrote: On 3/3/17 09:03, Tomas Vondra wrote: Attached is v2, fixing both issues. I wonder if + bytea *raw_page = PG_GETARG_BYTEA_P(0); + uargs->page = VARDATA(raw_page); is expected to work reliably, without copying the argument to a

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-06 Thread Peter Eisentraut
On 3/3/17 09:03, Tomas Vondra wrote: > Attached is v2, fixing both issues. I wonder if + bytea *raw_page = PG_GETARG_BYTEA_P(0); + uargs->page = VARDATA(raw_page); is expected to work reliably, without copying the argument to a different memory context. I think it would be better

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-04 Thread Peter Eisentraut
On 3/4/17 12:39, Tomas Vondra wrote: >> Can we have a test case for page_checksum(), or is that too difficult to >> get running deterministicly? > > I'm not sure it can be made deterministic. Certainly not on heap pages, > because then it'd be susceptible to xmin/xmax changes, but we have other

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-04 Thread Tomas Vondra
On 03/04/2017 02:08 PM, Peter Eisentraut wrote: On 3/3/17 09:03, Tomas Vondra wrote: Damn. In my defense, the patch was originally created for an older PostgreSQL version (to investigate issue on a production system), which used that approach to building values. Should have notice it, though.

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-04 Thread Peter Eisentraut
On 3/3/17 09:03, Tomas Vondra wrote: > Damn. In my defense, the patch was originally created for an older > PostgreSQL version (to investigate issue on a production system), which > used that approach to building values. Should have notice it, though. > > Attached is v2, fixing both issues.

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-03 Thread Tomas Vondra
On 03/03/2017 05:09 AM, Robert Haas wrote: On Mon, Feb 20, 2017 at 9:43 PM, Tomas Vondra wrote: BTW I've noticed the pageinspect version is 1.6, but we only have pageinspect--1.5.sql (and upgrade script to 1.6). Not sure that's entirely intentional? Actually,

Re: [HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-03-02 Thread Robert Haas
On Mon, Feb 20, 2017 at 9:43 PM, Tomas Vondra wrote: > BTW I've noticed the pageinspect version is 1.6, but we only have > pageinspect--1.5.sql (and upgrade script to 1.6). Not sure that's entirely > intentional? Actually, that's the New Way. See

[HACKERS] PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

2017-02-20 Thread Tomas Vondra
Hi, while investigating some checksum-related issues, I needed to perform some forensics on a copy of a btree page (taken from another instance using 'dd'). But I've ran into two pageinspect limitations, hopefully addressed by this patch: 1) bt_page_items(bytea) not defined We have