On Thu, Oct 1, 2015 at 8:13 PM, Nikolay Shaplov wrote:
> В письме от 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
:-)
pgindent would have taken care of that if needed. There is no need to add
noise in the code for this patch.
>> + <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.
I would completely remove this part.
> 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?
> Here is next patch in attachment.
Cool, thanks.
-test=# SELECT * FROM heap_page_items(get_raw_page('pg_class', 0));
+
+test=# select * from heap_page_items(get_raw_page('pg_range', 0));
This example in the docs is far too long in character length... We should
get that reduced.
+ Please notice that <function>t_bits</function> in tuple header
structure
+ is a binary bitmap, but <function>heap_page_items</function> returns
+ <function>t_bits</function> as human readable text representation of
+ original <function>t_bits</function> bitmap.
This had better remove the mention to "please notice". Still as this is
already described in htup_details.h there is no real point to describe it
again here: that's a bitmap of NULLs.
--
Michael
From 3a21fd817748b8001e91159c3f2a557088b4fa26 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Fri, 2 Oct 2015 12:58:13 +0900
Subject: [PATCH] Fix overestimated size of t_bits in pageinspect
In a tuple, an object-id field is added at the end of HeapTupleHeader
and the size of he header is updated to include it. However heap_page_items
in pageinspect ignored that fact and overestimated the size of bitmap of
NULLs by 4 bytes should an OID be defined in this tuple.
Per report from Nikolay Sharplov, for a problem found while working on
a new feature for pageinspect, and patch by Michael Paquier.
---
contrib/pageinspect/heapfuncs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 8d1666c..031d455 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -154,7 +154,6 @@ heap_page_items(PG_FUNCTION_ARGS)
lp_offset + lp_len <= raw_page_size)
{
HeapTupleHeader tuphdr;
- int bits_len;
/* Extract information from the tuple header */
@@ -180,9 +179,15 @@ heap_page_items(PG_FUNCTION_ARGS)
{
if (tuphdr->t_infomask & HEAP_HASNULL)
{
+ int bits_len;
+
bits_len = tuphdr->t_hoff -
offsetof(HeapTupleHeaderData, t_bits);
+ /* ignore OID field of tuple if present */
+ if (tuphdr->t_infomask & HEAP_HASOID)
+ bits_len -= sizeof(Oid);
+
values[11] = CStringGetTextDatum(
bits_to_text(tuphdr->t_bits, bits_len * 8));
}
--
2.6.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
