Fix incorrect computations of length of null bitmap in pageinspect.

Instead of using our standard macro for this calculation, this code
did it itself ... and got it wrong, leading to incorrect display of
the null bitmap in some cases.  Noted and fixed by Maksim Milyutin.

In passing, remove a uselessly duplicative error check.

Errors were introduced in commit d6061f83a; back-patch to 9.6
where that came in.

Maksim Milyutin, reviewed by Andrey Borodin

Discussion: https://postgr.es/m/ec295792-a69f-350f-6287-25a20e8f31d5@gmail.com

diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 8e15947a8187326351b2684aae3b362e319660d2..4dd620ee6f39f3fa401aa1e2da258d72c5d3b2ca 100644 (file)
--- 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 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 088254453eecd6293b843ea0ad4a8c8daaca5635..7438257c5bbc61e32f028dd2f3472b0f1f72afb0 100644 (file)
--- 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,24 +436,19 @@ 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)
-                       ereport(ERROR,
-                                       (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 493ca9b211d38db0105dbca6a3f28361c382a224..438e0351c41b895072dc72f6a962665692f53bec 100644 (file)
--- 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 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;