From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: pg_upgrade diffs on WIndows |
Date: | 2012-09-05 19:17:40 |
Message-ID: | 5047A554.6040906@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 09/05/2012 09:46 AM, Andrew Dunstan wrote:
>
> On 09/05/2012 09:11 AM, Bruce Momjian wrote:
>>
>> 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.
>
>
> I am quite happy not committing anything for now.
>
> There are two buildfarm members doing pg_upgrade tests: crake (Fedora
> 16) and pitta (Windows/Mingw64). The buildfarm code is experimental
> and not in any release yet, and when it is the test will be optional.
>
> The PG_BINARY_W change has only been verified on a non-buildfarm setup
> on my laptop (Mingw)
>
> Note that while it does look like there's a bug either in pg_upgrade
> or pg_dumpall, it's probably mostly harmless (adding some spurious CRs
> to function code bodies on Windows). I'd feel happier if it didn't,
> and happier still if I knew for sure the ultimate origin. Your
> pg_dumpall discovery above is interesting. I might have time later on
> today to delve into all this. I'm out of contact for the next few hours.
OK, I now have a complete handle on what's going on here, and withdraw
my earlier statement that I am confused on this issue :-)
First, one lot of CRs is produced because the pg_upgrade test script
calls pg_dumpall without -f and redirects that to a file, which Windows
kindly opens on text mode. The solution to that is to change the test
script to use pg_dumpall -f instead.
The second lot of CRs (seen in the second dump file in the diff i
previously sent) is produced by pg_upgrade writing its output in text
mode, which turns LF into CRLF. The solution to that is the patch to
dump.c I posted, which, as Bruce observed, does the same thing that
pg_dumpall does. Arguably, it should also open the input file in binary,
so that if there really is a CRLF in the dump it won't be eaten.
Another question is whether or not pg_dumpall (and pg_dump in text mode
too for that matter) should be trying to suppress newline translation on
its output even to stdout. It already does that for non-text formats
(see call to setmode()) but I don't see why we shouldn't for text as
well. But those are obviously longstanding bugs that we can leave to
another day.
cheers
andrew
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2012-09-05 19:36:26 | Re: pg_upgrade diffs on WIndows |
Previous Message | Robert Haas | 2012-09-05 18:02:50 | Re: [bugfix] sepgsql didn't follow the latest core API changes |