Re: pageinspect patch, for showing tuple data - Mailing list 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