Re: pageinspect patch, for showing tuple data

From: Nikolay Shaplov <n(dot)shaplov(at)postgrespro(dot)ru>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pageinspect patch, for showing tuple data
Date: 2015-10-02 09:57:35
Message-ID: 2330211.7MDZsYSnBv@nataraj-amd64
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

В письме от 2 октября 2015 13:10:22 Вы написали:

> > There also was a bug in original pageinspect, that did not get t_bits
>
> right
>
> > when there was OID in the tuple. I've fixed it too.
>
> Aha. Good catch! By looking at HeapTupleHeaderGetOid if the tuple has an
> OID it will be stored at the end of HeapTupleHeader and t_hoff includes it,
> so bits_len is definitely larger of 4 bytes should an OID be present.
> if (tuphdr->t_infomask & HEAP_HASNULL)
> {
> - bits_len = tuphdr->t_hoff -
> - offsetof(HeapTupleHeaderData, t_bits);
> + int bits_len =
> + ((tuphdr->t_infomask2 & HEAP_NATTS_MASK)/8 +
> 1)*8;
>
> values[11] = CStringGetTextDatum(
> - bits_to_text(tuphdr->t_bits,
> bits_len * 8));
> + bits_to_text(tuphdr->t_bits,
> bits_len));
> }
> And here is what you are referring to in your patch. I think that we should
> instead check for HEAP_HASOID and reduce bits_len by the size of Oid should
> one be present. As this is a bug that applies to all the existing versions
> of Postgres it would be good to extract it as a separate patch and then
> apply your own patch on top of it instead of including in your feature.
> Attached is a patch, this should be applied down to 9.0 I guess. Could you
> rebase your patch on top of it?
No we should not do it, because after t_bits there always goes padding bytes.
So OID or the top of tuple data is properly aligned. So we should not use
t_hoff for determinating t_bit's size at all.

Here is an example. I create a table with 10 columns and OID. (ten is greater
then 8, so there should be two bytes of t_bits data)

create table test3 (a1 int, a2 int, a3 int, a4 int,a5 int,a6 int, a7 int,a8 int, a9 int, a10 int) with oids;
insert into test3 VALUES
(1,2,3,4,5,6,7,8,null,10);

With your patch we get

test=# select lp, t_bits, t_data from heap_page_items(get_raw_page('test3', 0));
lp | t_bits | t_data
----+------------------------------------------+----------------------------------------------------------------------------
1 | 1111111101000000000000000000000000000000 | \x01000000020000000300000004000000050000000600000007000000080000000a000000
(1 row)

here we get 40 bits of t_bits.

With my way to calculate t_bits length we get

test=# select lp, t_bits, t_data from heap_page_items(get_raw_page('test3', 0));
lp | t_bits | t_data
----+------------------+----------------------------------------------------------------------------
1 | 1111111101000000 | \x01000000020000000300000004000000050000000600000007000000080000000a000000
(1 row)

16 bits as expected.

So I would keep my version of bits_len calculation

--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Oleg Bartunov 2015-10-02 10:17:32 Re: No Issue Tracker - Say it Ain't So!
Previous Message Kouhei Kaigai 2015-10-02 08:27:28 Re: Parallel Seq Scan