[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
Attachments:
pageinspect_tbits_alignment_fix.patchtext/x-patch; name=pageinspect_tbits_alignment_fix.patchDownload+1-1
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]https://github.com/postgres/postgres/blob/master/contrib/pageinspect/heapfuncs.c#L439 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
Attachments:
pageinspect_tbits_alignment_fix_v2.patchtext/plain; charset=UTF-8; name=pageinspect_tbits_alignment_fix_v2.patch; x-mac-creator=0; x-mac-type=0Download+30-5
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