On 16 August 2017 at 23:14, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Aug 15, 2017 at 4:36 PM, Tomas Vondra > <tomas.von...@2ndquadrant.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. > > For other multi-purpose flags we have macros, and I think it'd make sense to use them here too. Eschew direct use of HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID and HEAP_XMIN_FROZEN in tests. Instead, consistently use HeapXminIsFrozen(), HeapXminIsCommitted(), and HeapXminIsInvalid() or something like that. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
