I had a quick look into this patch and also tested it and following are my observations.
1) I am seeing a server crash when passing any non meaningful value for t_infomask2 to heap_infomask_flags().
postgres=# SELECT heap_infomask_flags(2816, 3); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. !> \q
Following is the backtrace,
(gdb) bt #0 0x0000000000d9c55b in pg_detoast_datum (datum=0x0) at fmgr.c:1833 #1 0x0000000000b87374 in construct_md_array (elems=0x2ad74c0, nulls=0x0, ndims=1, dims=0x7ffc0b0bbcd0, lbs=0x7ffc0b0bbcc0, elmtype=25, elmlen=-1, elmbyval=0 '\000', elmalign=105 'i') at arrayfuncs.c:3382 #2 0x0000000000b8709f in construct_array (elems=0x2ad74c0, nelems=10, elmtype=25, elmlen=-1, elmbyval=0 '\000', elmalign=105 'i') at arrayfuncs.c:3316 #3 0x00007fb8001603a5 in heap_infomask_flags (fcinfo=0x2ad3b88) at heapfuncs.c:597
Fixed.
2) I can see the documentation for heap_infomask(). But, I do not see it being defined or used anywhere in the patch.
+ <para> + The <function>heap_infomask</function> function can be used to unpack the + recognised bits of the infomasks of heap tuples. + </para>
Fixed. Renamed the function, missed a spot.
3) If show_combined flag is set to it's default value and a tuple is frozen then may i know the reason for not showing it as frozen tuple when t_infomask2 is passed as zero.
It was a consequence of (1). Fixed.
4) I think, it would be better to use the same argument name for the newly added function i.e heap_infomask_flags() in both documentation and sql file. I am basically refering to 'include_combined' argument. IF you see the function definition, the argument name used is 'include_combined' whereas in documentation you have mentioned 'show_combined'.
Fixed, thanks.
I want to find time to expand the tests on this some more and look more closely, but here it is for now.
