On 01/11/14 14:00, Michael Paquier wrote:
More comments: - Heikki already mentioned it, but after reading the code I see little point in having the extra field implementing like that in core for many reasons even if it is *just* 4 bytes: 1) It is untested and actually there is no direct use for it in core. 2) Pushing code that we know as dead is no good, that's a feature more or less defined as maybe-useful-but-we-are-not-sure-yet-what-to-do-with-it. 3) If you're going to re-use this API in BDR, which is a fork of Postgres. You'd better complete this API in BDR by yourself and not bother core with that. For those reasons I think that this extra field should be ripped off from the patch.
Well this is not BDR specific thing, the idea is that with logical replication, commit timestamp is not enough for conflict handling, you also need to have additional info in order to identify some types of conflicts conflicts (local update vs remote update for example). So the extradata field was meant as something that could be used to add the additional info to the xid.
But I see your point, I think solving this issue can be left to the replication identifier patch that is discussed in separate thread.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
