On 06/24/2015 09:43 AM, Michael Paquier wrote:
> Attached is a new set of patches. Except for the last ones that
> addresses one issue of pg_rewind (symlink management when streaming
> PGDATA), all the others introduce if_not_exists options for the
> functions of genfile.c. The pg_rewind stuff could be more polished
> though. Feel free to comment.
I've committed the additional option to the functions in genfile.c (I
renamed it to "missing_ok", for clarity), and the pg_rewind changes to
use that option.
I ended up refactoring the patch quite a bit, so if you could
double-check what I committed to make sure I didn't break anything, that
would be great.
I didn't commit the tablespace or symlink handling changes yet, will
review those separately.
I also didn't commit the new regression test yet. It would indeed be
nice to have one, but I think it was a few bricks shy of a load. It
should work in a freshly initdb'd system, but not necessarily on an
existing installation. First, it relied on the fact that
postgresql.conf.auto exists, but a DBA might remove that if he wants to
make sure the feature is not used. Secondly, it relied on the fact that
pg_twophase is empty, but there is no guarantee of that either. Third,
the error messages included in the expected output, e.g "No such file or
directory", depend on the operating system and locale. And finally, it'd
be nice to test more things, in particular the behaviour of different
offsets and lengths to pg_read_binary_file(), although an incomplete
test would be better than no test at all.
- Heikki
