On Tue, Dec 16, 2014 at 10:26 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> In any case, I have a couple of comments about this patch as-is:
> - If the move to src/bin is done, let's update the MSVC scripts accordingly
> - contrib/pg_rewind/.gitignore should be cleaned up from its unnecessary entries
> - README is incorrect, it is still mentioned for example that this
> code should be copied inside PostgreSQL code tree as contrib/pg_rewind
(Sorry email got sent...)
- Code is going to need a brush to clean up things of this type:
+ PG_9.4_201403261
+ printf("Report bugs to https://github.com/vmware/pg_rewind.\n");
- Versioning should be made the Postgres-way, aka not that:
+#define PG_REWIND_VERSION "0.1"
But a way similar to other utilities:
pg_rewind (PostgreSQL) 9.5devel
- Shouldn't we use $(SHELL) here at least?
+++ b/contrib/pg_rewind/launcher
@@ -0,0 +1,6 @@
+#!/bin/bash
+#
+# Normally, psql feeds the files in sql/ directory to psql, but we want to
+# run them as shell scripts instead.
+
+bash
I doubt that this would work for example with MinGW.
- There are a couple of TODO items which may be good to fix:
+ *
+ * TODO: This assumes that there are no timeline switches on the target
+ * cluster after the fork.
+ */
and:
+ /*
+ * TODO: move old file out of the way, if any. And perhaps create the
+ * file with temporary name first and rename in place after it's done.
+ */
- Regression tests, which have a good coverage btw are made using
shell scripts. There is some initialization process that could be
refactored IMO as this code is duplicated with pg_upgrade. For
example, listen_addresses initialization has checks fir MINGW,
environment variables PG* are unset, etc.
Regards,
--
Michael