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

On Wed, Sep 11, 2019 at 1:46 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> 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.

Hmm it will be more consistent with other functions but I think we
would need to increase the pageinspect version to 1.8 and need the new
sql file to rename the function name. And it will be for PG12, not
PG13. If we have to do it someday I think it's better to do it in PG12
that the table AM has been introduced to. Anyway I've attached
separate patch for 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.
>

Seems good idea.

> 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.

I've reconsidered and updated the regression tests.

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

Fixed.

I've attached the updated patch that incorporated all comments. I kept
the function as superuser-restricted.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center