On Thu, Dec 19, 2013 at 5:44 AM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2013-12-18 21:42:25 -0500, Robert Haas wrote: >> On Wed, Dec 18, 2013 at 5:54 PM, Andres Freund <and...@2ndquadrant.com> >> wrote: >> >> if (frz->frzflags & XLH_FREEZE_XVAC) >> >> + { >> >> HeapTupleHeaderSetXvac(tuple, FrozenTransactionId); >> >> + /* If we somehow haven't hinted the tuple previously, do it >> >> now. */ >> >> + HeapTupleHeaderSetXminCommitted(tuple); >> >> + } >> > >> > What's the reasoning behind adding HeapTupleHeaderSetXminCommitted() >> > here? >> >> I'm just copying the existing logic. See the final stanza of >> heap_prepare_freeze_tuple. > > Yes, but why don't you keep that in heap_prepare_freeze_tuple()? Just > because of HeapTupleHeaderSetXminCommitted()?
Yes, that's pretty much it. > I dislike transporting the > infomask in the wal record and then changing it away from that again > afterwards. I don't really see a problem with it. Relying on the macros to tweak the bits seems more future-proof than inventing some other way to do it (that might even get copied into other parts of the code where it's even less safe). I actually think transporting the infomask is kind of a funky way to handle this in the first instance, but I don't think it's this patch's job to kibitz that decision. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers