I don't think so -- the "committed" and "invalid" meanings are effectively canceled when the "frozen" mask is present.
I mean, "committed" and "invalid" contradict each other...
FWIW I agree with Craig - the functions should output the masks raw, without any filtering. The reason is that when you're investigating data corruption or unexpected behavior, all this is very useful when reasoning about what might (not) have happened.
Or at least make the filtering optional.
I don't think "filtering" is the right way to think about it. It's just labeling each combination of bits with the meaning appropriate to that combination of bits.
I mean, if you were displaying the contents of a CLOG entry, would you want the value 3 to be displayed as COMMITTED ABORTED SUBCOMMITTED because TRANSACTION_STATUS_COMMITTED|TRANSACTION_STATUS_ABORTED == TRANSACTION_STATUS_SUB_COMMITTED?
I realize that you may be used to thinking of the HEAP_XMIN_COMMITTED and HEAP_XMAX_COMMITTED bits as two separate bits, but that's not really true any more. They're a 2-bit field that can have one of four values: committed, aborted, frozen, or none of the above.
All I'm saying is that having the complete information (knowing which bits are actually set in the bitmask) is valuable when reasoning about how you might have gotten to the current state. Which I think is what Craig is after.
What I think we should not do is interpret the bitmasks (omitting some of the information) assuming all the bits were set correctly.
I agree, and the patch already does half of this: it can output just the raw bit flags, or it can interpret them to show HEAP_XMIN_FROZEN etc.
So the required change, which seems to have broad agreement, is to have the "interpret the bits" mode show only HEAP_XMIN_FROZEN when it sees HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID, etc. We can retain raw-flags output as-is for when seriously bogus state is suspected.