I am still planning to do more extensive tests, and study a bit more committs.c (with even more comments) as it is the core part of the feature.
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.
- The API to get the commit timestamp is not that user-friendly, and I think it could really be improved, to something like that for example:
pg_get_commit_timestamp(from_xact xid, number_of_xacts int); pg_get_commit_timestamp(from_xact xid); pg_get_commit_timestamp(); or pg_get_latest_commit_timestamp();
from_xact to NULL means latest. number_of_xacts to NULL means 1.
Comment in line with the fact that extra field is well, not really useful.
