Re: [PATCH] pageinspect function to decode infomasks

I checked for code related to infomask.
(add flag state -- HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID, HEAP_XMIN_FROZEN)

first i'm still beginner level about postgresql, so my opinion may be wrong.

if the "HEAP_XMIN_COMMITTED" flag is added, check the function of "HeapTupleHeaderXminInvalid"
if the "HEAP_XMIN_INVALID" flag is added, check the function of "HeapTupleHeaderXminCommitted"
if the "HEAP_XMIN_FROZEN" flag is added, use the "HeapTupleHeaderSetXminFrozen" function or
use the code as
--------------------------------------
xid = HeapTupleHeaderGetXmin(tuple);
if (TransactionIdIsNormal(xid))
{
if (TransactionIdPrecedes(xid, cutoff_xid))
{
frz->t_infomask |= HEAP_XMIN_FROZEN;
changed = true;
}
else
totally_frozen = false;
}
--------------------------------------
to add the flag.

so as a result, HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED is cannot coexist.
unfortunately, i don't know if HEAP_XMIN_COMMITTED and HEAP_XMIN_FROZEN flags can coexist.

so i think it's also a good idea to output the raw masks, without any filtering.
however, i think the information that is presented to the user should inform us which flags was entered.

Regards.
Moon

> -----Original Message-----
> From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Tomas Vondra
> Sent: Wednesday, August 16, 2017 5:36 AM
> To: Robert Haas
> Cc: pgsql-hackers(at)postgresql(dot)org
> Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks
>
>
>
> On 08/15/2017 09:55 PM, Robert Haas wrote:
> > On Tue, Aug 15, 2017 at 3:42 PM, Tomas Vondra
> > <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
> >> What I think we should not do is interpret the bitmasks (omitting
> >> some of the information) assuming all the bits were set correctly.
> >
> > I'm still confused. HEAP_XMIN_COMMITTED|HEAP_XMIN_ABORTED ==
> > HEAP_XMIN_FROZEN. Nobody is proposing to omit anything; to the
> > contrary, what's being proposed is not to display the same thing twice
> > (and in a misleading fashion, to boot).
> >
>
> I understand your point. Assume you're looking at this bit of code:
>
> if (HeapTupleHeaderXminCommitted(enumval_tup->t_data))
> return;
>
> which is essentially
>
> if (enumval_tup->t_data & HEAP_XMIN_COMMITTED)
> return;
>
> If the function only gives you HEAP_XMIN_FROZEN, how likely is it you miss
> this actually evaluates as true?
>
> You might say that people investigating issues in this area of code should
> be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right ...
>
> regards
>
> --
> Tomas Vondra http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org) To make
> changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers