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