On Tue, Sep 29, 2015 at 11:39 PM, Nikolay Shaplov wrote:
> But since now we actually parse data with tuple_data_split, we can
provide a
> precisely formed  fake information, so you are not limited with how it is
> actually stored in page. You just pass any arguments you want. So you
does not
> need warning mode anymore.

Yeah, I agree with you here, let's simplify it then. One could as well
catch the error in a plpgsql wrapper function if that's really necessary
and log the failed events at the same time in a custom way.

-                                 errmsg("input page too small (%d bytes)",
+                                        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.

+     <para>
+     General idea about output columns: <function>lp_*</function>
+     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
+     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
+     not. Please notice that t_bits here is not an actual value that is
+     in tuple data, but it's text representation  with '0' and '1'
+     </para>
I would remove that as well. htup_details.h contains all this information.

+     <para>
+     For more detailed information see documentation:
+     <xref linkend="storage-page-layout">, <xref
+     and source code: <filename>src/include/storage/itemid.h</>,
+     <filename>src/include/access/htup_details.h</>.
+     </para>
This looks cool to me though.

+test=# select * from heap_page_item_attrs(get_raw_page('pg_range',
+[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

+      <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.

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

+                       values[13] = PointerGetDatum(tuple_data_bytea);
+                       nulls[13] = false;
There is no point to set nulls[13] here.

+test=# select tuple_data_split('pg_range'::regclass,
'\x400f00001700000000000000ba0700004a0f0000520f0000'::bytea, 2304, 6, null);
+                                   tuple_data_split
+(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.

Reply via email to