Thread: [HACKERS] wrong t_bits alignment in pageinspect
Hi! I found out the problem in exposing values of t_bits field from heap_page_items function. When the number of attributes in table is multiple of eight, t_bits column shows double number of bits in which data fields are included. For example: # create table tbl(f1 int, f2 int, f3 int, f4 int, f5 int, f6 int, f7 int, f8 int); # insert into tbl(f1, f8) values (x'f1'::int, 0); # select t_bits, t_data from heap_page_items(get_raw_page('tbl', 0)); t_bits | t_data ------------------+-------------------- 1000000110001111 | \xf100000000000000 I suppose the prefix 10000001 corresponds to real value of t_bits, the rest part 10001111 - to the lower byte of f1 field of tbl. Attached patch fixes this issue. -- Regards, Maksim Milyutin
Attachment
Hi! > 15 дек. 2017 г., в 18:53, Maksim Milyutin <milyutinma@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? Also, may be macro HeapTupleHeaderGetNatts() will be a little bit easier to read. Best regards, Andrey Borodin. [0] https://github.com/postgres/postgres/blob/master/contrib/pageinspect/heapfuncs.c#L439
Hi! 02.01.2018 12:33, Andrey Borodin wrote: >> 15 дек. 2017 г., в 18:53, Maksim Milyutin <milyutinma@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
Attachment
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: not tested Spec compliant: tested, passed Documentation: tested, passed Everything is fixed, works properly and I have no new notices. I think that patch is ready for committer. Best regards, Andrey Borodin. The new status of this patch is: Ready for Committer
Maksim Milyutin <milyutinma@gmail.com> writes: > Attached a new version of patch with regression test. Looks good, pushed. I also removed one error check from tuple_data_split --- there doesn't seem to be a lot of point in checking that bits_str_len is some multiple of 8 when we're about to check that it's equal to a particular multiple of 8. regards, tom lane