Re: pageinspect patch, for showing tuple data - Mailing list pgsql-hackers

В письме от 30 сентября 2015 13:49:00 пользователь Michael Paquier написал:



>
> -                                 errmsg("input page too small (%d bytes)",
> raw_page_size)));
> +                                        errmsg("input page too small (%d
> bytes)", raw_page_size)));
> Please be careful of spurious diffs. Those just make the life of committers
> more difficult than it already is.

Oh, that's not diff. That's I've fixed indent on the code I did not write :-)


> +     <para>
> +     General idea about output columns: <function>lp_*</function>
> attributes
> +     are about where tuple is located inside the page;
> +     <function>t_xmin</function>, <function>t_xmax</function>,
> +     <function>t_field3</function>, <function>t_ctid</function> are about
> +     visibility of this tuplue inside or outside of the transaction;
> +     <function>t_infomask2</function>, <function>t_infomask</function> has
> some
> +     information about properties of attributes stored in tuple data.
> +     <function>t_hoff</function> sais where tuple data begins and
> +     <function>t_bits</function> sais which attributes are NULL and which
> are
> +     not. Please notice that t_bits here is not an actual value that is
> stored
> +     in tuple data, but it's text representation  with '0' and '1'
> charactrs.
> +     </para>
> I would remove that as well. htup_details.h contains all this information.
Made it even more shorter. Just links and comments about representation of
t_bits.


> +<screen>
> +test=# select * from heap_page_item_attrs(get_raw_page('pg_range',
> 0),'pg_range'::regclass);
> +[loooooong tuple data]
> This example is too large in character per lines, I think that you should
> cut a major part of the fields, why not just keeping lp and t_attrs for
> example.
Did id.

>
> +      <tbody>
> +       <row>
> +        <entry><structfield>rel_oid</structfield></entry>
> +        <entry><type>oid</type></entry>
> +        <entry>OID of the relation, of the tuple we want to split</entry>
> +       </row>
> +
> +       <row>
> +        <entry><structfield>tuple_data</structfield></entry>
> +        <entry><type>bytea</type></entry>
> +        <entry>tuple raw data to split
> +        </entry>
> +       </row>
> In the description of tuple_data_split, I am not sure it is worth listing
> all the argument of the function like that. IMO, we should just say: those
> are the fields returned by "heap_page_items". tuple_data should as well be
> renamed to t_data.
Did it.

>
> +      tuple attributes instead of one peace of raw tuple data. All other
> return
> This is not that "peaceful" to me. It should be "piece" :)
Oops ;-)

> +                       values[13] = PointerGetDatum(tuple_data_bytea);
> +                       nulls[13] = false;
> There is no point to set nulls[13] here.
Oh, you are right!

>
> +<screen>
> +test=# select tuple_data_split('pg_range'::regclass,
> '\x400f00001700000000000000ba0700004a0f0000520f0000'::bytea, 2304, 6, null);
> +                                   tuple_data_split
> +---------------------------------------------------------------------------
> ------------ +
> {"\\x400f0000","\\x17000000","\\x00000000","\\xba070000","\\x4a0f0000","\\x5
> 20f0000"} +(1 row)
> This would be more demonstrative if combined with heap_page_items, like
> that for example:
> SELECT tuple_data_split('pg_class'::regclass, t_data, t_infomask,
> t_infomask2, t_bits) FROM heap_page_items(get_raw_page('pg_class', 0));
> And actually this query crashes.
Oh... It crached because I did not check that t_data can actually be NULL.

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.


Here is next patch in attachment.


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