Re: [HACKERS] [PATCH] pageinspect function to decode infomasks

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Alvaro Herrera from 2ndQuadrant <alvherre@alvh.no-ip.org>, Craig Ringer <craig@2ndquadrant.com>, Ashutosh Sharma <ashu.coek88@gmail.com>, Julien Rouhaud <rjuju123@gmail.com>, Peter Geoghegan <pg@bowt.ie>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Abhijit Menon-Sen <ams@2ndquadrant.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Pavan Deolasee <pavan.deolasee@2ndquadrant.com>
Date: 2019-09-11T05:45:51Z
Lists: pgsql-hackers
On Tue, Sep 10, 2019 at 08:29:43AM +0530, Amit Kapila wrote:
> Good thought, but I think even if we want to change the name of
> tuple_data_split, it might be better done separately.

Yes, that's not the problem of this patch.  Not sure if it actually
makes sense either to change it.

The regression tests added are rather unreadable when it comes to
print a lot of infomask flags.  Could you add at least some unnest()
calls to the queries using heap_infomask_flags()?  It would make the
diff lookup much more straight-forward to understand.

It would be good to comment as well what 2816 and 1080 stand for.  The
current code makes it hard to understand for which purpose this is
used in the tests.

+      If decode_combined is set, combination flags like
Missing a markup here.

I am switching the patch as "waiting on author".  Could you address
the comments raised please?
--
Michael

Commits

  1. Redesign pageinspect function printing infomask bits

  2. Add to pageinspect function to make t_infomask/t_infomask2 human-readable

  3. Improve documentation of pageinspect

  4. Improve pageinspect module