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

On Thu, Sep 5, 2019 at 2:17 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Thanks. I hope the attached new patch fixes this issue.
>

*
+-- decode infomask flags as human readable flag names
+CREATE FUNCTION heap_infomask_flags(
+ infomask integer,
+ infomask2 integer,
+ decode_combined boolean DEFAULT false)
+RETURNS text[]
+AS 'MODULE_PATHNAME', 'heap_infomask_flags'

Isn't it better to name this function as tuple_infomask_flags or
something like that? The other functions that start their name with
heap take page as input. Also, I think the index-related functions
that start with index name take blk/page as input.

*
+ <varlistentry>
+ <term>
+ <function>heap_infomask_flags(infomask integer, infomask2
integer, decode_combined bool) returns text[]</function>
+ <indexterm>
+ <primary>heap_infomask_flags</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <function>heap_infomask_flags</function> decodes the
+ <structfield>t_infomask</structfield> and
+ <structfield>t_infomask2</structfield> returned by
+ <function>heap_page_items</function> into a human-readable
+ array of flag names. This can be used to see the tuple hint bits etc.
+ </para>

I think it is better to use one example for this function as we do for
other functions in this doc.

*
+ <para>
+ If decode_combined is set, combination flags like
+ <literal>HEAP_XMIN_FROZEN</literal> are output instead of raw
+ flags, <literal>HEAP_XMIN_COMMITTED</literal> and
+ <literal>HEAP_XMIN_INVALID</literal>. Default value is
+ <literal>false</literal>.
+ </para>

decode_combined should use parameter marker (like
<parameter>decode_combined</parameter>). Instead of saying
"decode_combined" is set, you can use true/false terminology as that
is what we use for this parameter. See explanation of "do_detoast"
that is used in function tuple_data_split in the same doc
(pageinspect.sgml) for reference.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com