>Cool, thanks. I have gone through your patch in details, and updated >it as the attached. Here are some comments. >'8000' as OID for the new function was not really random, so to be >fair with the other patches, I picked up the first random value >unused_oids has given me instead. > >There were some indentation issues, and pgindent got that fixed. >I think that it is better to use "replication origin" in the docs >instead of just origin. I have kept "origin" in the functions for >now as that sounded cleaner to me, but we may consider using something >like "reporigin" as well as attribute name. > >The tests could just use tstzrange() to make sure that the timestamps >have valid values, so I have switched to that, and did not resist to >do the same in the existing tests. > >+-- Test when it can not find the transaction >+SELECT * FROM pg_xact_commit_timestamp_origin((:'txid_set_origin'::text::int + >10)::text::xid) x; >This test could become unstable, particularly if it gets used in a >parallel environment, so I have removed it. Perhaps I am just >over-pessimistic here though.. > >As a side note, I think that we could just remove the alternate output >of commit_ts/, as it does not really get used because of the >NO_INSTALLCHECK present in the module's Makefile. That would be the >job of a different patch, so I have updated it accordingly. Glad to >see that you did not forget to adapt it in your own patch. > >(The change in catversion.h is a self-reminder...) Thanks for all of that, so many details I still need to pay attention when submit a patch.
Regards, Highgo Software (Canada/China/Pakistan) URL : www.highgo.ca EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
