Hi, I don't know how well I can review the 0001 (the TAP infra patch) but it looks okay to me.
I don't really have any complaints about 0002 either. I like that it's more or less one self-contained function and there are no weird ifdefs anymore like in 9.6 version (btw your commit message talks about 9.5 but it was 9.6). I also like the clever test :) I am slightly worried about impact of the readTimeLineHistory() call but I think it should be called so little that it should not matter. That brings us to the big patch 0003. I still don't like the "New in 10.0" comments in documentation, for one it's 10, not 10.0 and mainly we don't generally write stuff like this to documentation, that's what release notes are for. There is large amounts of whitespace mess (empty lines with only whitespace, spaces at the end of the lines), nothing horrible, but should be cleaned up. One thing I don't understand much is the wal_level change and turning off catalogXmin tracking. I don't really see anywhere that the catalogXmin would be reset in control file for example. There is TODO in UpdateOldestCatalogXmin() that seems related but tbh I don't follow what's happening there - comment says about not doing anything, but the code inside the if block are only Asserts. -- 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
