Re: [PATCH] pageinspect function to decode infomasks

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] pageinspect function to decode infomasks
Date: 2017-08-16 15:14:03
Message-ID: CA+TgmoboDNqZvL-AdbwK+Joxg=MPnDeMdzqBpydaaO7s0d27UQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 15, 2017 at 4:36 PM, Tomas Vondra
<tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
> 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 ...

Yes, I think that's what I would say. I mean, if you happen to NOT
know that committed|invalid == frozen, but you DO know what committed
means and what invalid means, then you're going to be *really*
confused when you see committed and invalid set on the same tuple.
Showing you frozen has got to be clearer.

Now, I agree with you that a test like (enumval_tup->t_data &
HEAP_XMIN_COMMITTED) could be confusing to someone who doesn't realize
that HEAP_XMIN_FROZEN & HEAP_XMIN_COMMITTED == HEAP_XMIN_COMMITTED,
but I think that's just one of those things that unfortunately is
going to require adequate knowledge for people investigating issues.
If there's an action item there, it might be to try to come up with a
way to make the source code clearer.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-08-16 15:16:58 Re: Atomics for heap_parallelscan_nextpage()
Previous Message Aleksander Alekseev 2017-08-16 15:10:29 Re: SCRAM salt length