Hi!

02.01.2018 12:33, Andrey Borodin wrote:
15 дек. 2017 г., в 18:53, Maksim Milyutin <milyuti...@gmail.com> написал(а):

I found out the problem in exposing values of t_bits field from heap_page_items 
function.
Probably, this [0] place contains similar bug too?

[0] 
https://github.com/postgres/postgres/blob/master/contrib/pageinspect/heapfuncs.c#L439

Yes, it's so. Thanks a lot for notice.

Attached a new version of patch with regression test.

--
Regards,
Maksim Milyutin

diff --git a/contrib/pageinspect/expected/page.out 
b/contrib/pageinspect/expected/page.out
index 8e15947a81..97af6f5bd2 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -92,3 +92,20 @@ create table test_part1 partition of test_partitioned for 
values from ( 1 ) to (
 select get_raw_page('test_part1', 0); -- get farther and error about empty 
table
 ERROR:  block number 0 is out of range for relation "test_part1"
 drop table test_partitioned;
+-- check null bitmap alignment for table whose the number of attributes is 
multiple of 8
+create table test8 (f1 int, f2 int, f3 int, f4 int, f5 int, f6 int, f7 int, f8 
int);
+insert into test8(f1, f8) values (x'f1'::int, 0);
+select t_bits, t_data from heap_page_items(get_raw_page('test8', 0));
+  t_bits  |       t_data       
+----------+--------------------
+ 10000001 | \xf100000000000000
+(1 row)
+
+select tuple_data_split('test8'::regclass, t_data, t_infomask, t_infomask2, 
t_bits)
+    from heap_page_items(get_raw_page('test8', 0));
+                      tuple_data_split                       
+-------------------------------------------------------------
+ {"\\xf1000000",NULL,NULL,NULL,NULL,NULL,NULL,"\\x00000000"}
+(1 row)
+
+drop table test8;
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 088254453e..99f409846c 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -234,7 +234,7 @@ heap_page_items(PG_FUNCTION_ARGS)
                                        int                     bits_len;
 
                                        bits_len =
-                                               ((tuphdr->t_infomask2 & 
HEAP_NATTS_MASK) / 8 + 1) * 8;
+                                               
BITMAPLEN(HeapTupleHeaderGetNatts(tuphdr)) * BITS_PER_BYTE;
                                        values[11] = CStringGetTextDatum(
                                                                                
                         bits_to_text(tuphdr->t_bits, bits_len));
                                }
@@ -436,12 +436,12 @@ tuple_data_split(PG_FUNCTION_ARGS)
                int                     bits_str_len;
                int                     bits_len;
 
-               bits_len = (t_infomask2 & HEAP_NATTS_MASK) / 8 + 1;
+               bits_len = BITMAPLEN(t_infomask2 & HEAP_NATTS_MASK) * 
BITS_PER_BYTE;
                if (!t_bits_str)
                        ereport(ERROR,
                                        (errcode(ERRCODE_DATA_CORRUPTED),
                                         errmsg("argument of t_bits is null, 
but it is expected to be null and %d character long",
-                                                       bits_len * 8)));
+                                                       bits_len)));
 
                bits_str_len = strlen(t_bits_str);
                if ((bits_str_len % 8) != 0)
@@ -449,11 +449,11 @@ tuple_data_split(PG_FUNCTION_ARGS)
                                        (errcode(ERRCODE_DATA_CORRUPTED),
                                         errmsg("length of t_bits is not a 
multiple of eight")));
 
-               if (bits_len * 8 != bits_str_len)
+               if (bits_len != bits_str_len)
                        ereport(ERROR,
                                        (errcode(ERRCODE_DATA_CORRUPTED),
                                         errmsg("unexpected length of t_bits 
%u, expected %d",
-                                                       bits_str_len, bits_len 
* 8)));
+                                                       bits_str_len, 
bits_len)));
 
                /* do the conversion */
                t_bits = text_to_bits(t_bits_str, bits_str_len);
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 493ca9b211..16b4571fbf 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -41,3 +41,11 @@ select get_raw_page('test_partitioned', 0); -- error about 
partitioned table
 create table test_part1 partition of test_partitioned for values from ( 1 ) to 
(100);
 select get_raw_page('test_part1', 0); -- get farther and error about empty 
table
 drop table test_partitioned;
+
+-- check null bitmap alignment for table whose the number of attributes is 
multiple of 8
+create table test8 (f1 int, f2 int, f3 int, f4 int, f5 int, f6 int, f7 int, f8 
int);
+insert into test8(f1, f8) values (x'f1'::int, 0);
+select t_bits, t_data from heap_page_items(get_raw_page('test8', 0));
+select tuple_data_split('test8'::regclass, t_data, t_infomask, t_infomask2, 
t_bits)
+    from heap_page_items(get_raw_page('test8', 0));
+drop table test8;

Reply via email to