Re: pg_upgrade diffs on WIndows

On Tue, Sep 4, 2012 at 03:44:35PM -0400, Andrew Dunstan wrote:
>
> On 09/04/2012 03:09 PM, Andrew Dunstan wrote:
> >I realized this morning that I might have been a bit cavalier in
> >using dos2unix to smooth away differences in the dumpfiles
> >produced by pg_upgrade. Attached is a dump of the diff if this
> >isn't done, with Carriage Returns printed as '*' to make them
> >visible. As can be seen, in function bodies dump2 has the Carriage
> >Returns doubled. I have not had time to delve into how this comes
> >about, and I need to attend to some income-producing activity for
> >a bit, but I'd like to get it cleaned up ASAP. We are under the
> >hammer for 9.2, so any help other people can give on this would be
> >appreciated.
> >
>
>
> Actually, I have the answer - it's quite simple. We just need to
> open the output files in binary mode when we split the dumpall file.
> The attached patch fixes it. I think we should backpatch the first
> part to 9.0.

> diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
> index b905ab0..0a96dde 100644
> --- a/contrib/pg_upgrade/dump.c
> +++ b/contrib/pg_upgrade/dump.c
> @@ -62,10 +62,10 @@ split_old_dump(void)
> if ((all_dump = fopen(filename, "r")) == NULL)
> pg_log(PG_FATAL, "Could not open dump file \"%s\": %s\n", filename, getErrorText(errno));
> snprintf(filename, sizeof(filename), "%s", GLOBALS_DUMP_FILE);
> - if ((globals_dump = fopen_priv(filename, "w")) == NULL)
> + if ((globals_dump = fopen_priv(filename, PG_BINARY_W)) == NULL)
> pg_log(PG_FATAL, "Could not write to dump file \"%s\": %s\n", filename, getErrorText(errno));
> snprintf(filename, sizeof(filename), "%s", DB_DUMP_FILE);
> - if ((db_dump = fopen_priv(filename, "w")) == NULL)
> + if ((db_dump = fopen_priv(filename, PG_BINARY_W)) == NULL)
> pg_log(PG_FATAL, "Could not write to dump file \"%s\": %s\n", filename, getErrorText(errno));
>
> current_output = globals_dump;
> diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
> index d411ac6..3899600 100644
> --- a/contrib/pg_upgrade/test.sh
> +++ b/contrib/pg_upgrade/test.sh
> @@ -128,10 +128,6 @@ else
> sh ./delete_old_cluster.sh
> fi
>
> -if [ $testhost = Msys ] ; then
> - dos2unix "$temp_root"/dump1.sql "$temp_root"/dump2.sql
> -fi
> -
> if diff -q "$temp_root"/dump1.sql "$temp_root"/dump2.sql; then
> echo PASSED
> exit 0

I reviewed this idea and supports this patch's inclusion in 9.2. I was
unclear why it was needed, but I see pg_dumpall, which is the file
pg_upgrade splits apart, as also using binary mode to write this file:

OPF = fopen(filename, PG_BINARY_W);

I agree with Tom that pg_upgrade needs some quiet time. ;-) Andrew,
have a sufficient number of buildfarm members verified our recent
patches that this can be added. My patch from last night was mostly C
comments so isn't something that needs testing.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +