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

On Tue, Sep 10, 2019 at 10:21 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Sep 9, 2019 at 6:22 PM Alvaro Herrera from 2ndQuadrant
> <alvherre@alvh.no-ip.org> wrote:
> >
> > On 2019-Sep-08, Amit Kapila wrote:
> >
> > > On Thu, Sep 5, 2019 at 2:17 PM Masahiko Sawada <sawada.mshk@gmail.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.
> >
> > I think that other table AMs are not necessarily going to use the same
> > infomask flags, so I think we should keep a name that is somehow
> > heapam-specific.  Maybe "heapam_infomask_flags" would be okay?
> >
>
> It will look bit strange to use heapam as a prefix for this function
> when all others use heap.  I guess if we want to keep it AM specific,
> then the proposed name (heap_infomask_flags) is better or
> alternatively we can consider heap_tuple_infomask_flags?

+1 for heap_tuple_infomask_flags. And do we need to change
tuple_data_split to heap_tuple_data_split as well because it's also a
heap specific function?

Regards,

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