On Wed, May 3, 2017 at 10:44 PM, Ashutosh Sharma <ashu.coe...@gmail.com> wrote:
> Hi Craig, > > On Wed, May 3, 2017 at 10:50 AM, Craig Ringer <cr...@2ndquadrant.com> > wrote: > > On 3 May 2017 at 12:32, Haribabu Kommi <kommi.harib...@gmail.com> wrote: > >> [Adding -hackers mailing list] > >> > >> On Fri, Apr 28, 2017 at 6:28 PM, <henry_boehl...@agilent.com> wrote: > >>> > >>> The following bug has been logged on the website: > >>> > >>> Bug reference: 14634 > >>> Logged by: Henry Boehlert > >>> Email address: henry_boehl...@agilent.com > >>> PostgreSQL version: 9.6.2 > >>> Operating system: Windows Server 2012 R2 6.3.9600 > >>> Description: > >>> > >>> Executing command pg_basebackup -D -F t writes its output to stdout, > which > >>> is open in text mode, causing LF to be converted to CR LF thus > corrupting > >>> the resulting archive. > >>> > >>> To write the tar to stdout, on Windows stdout's mode should be > temporarily > >>> switched to binary. > >>> > >>> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx > >> > >> > >> Thanks for reporting the issue. > >> With the attached patch, I was able to extract the tar file that gets > >> generated when the tar file is written into stdout. I tested the > >> the compressed tar also. > >> > >> This bug needs to be fixed in back branches also. > > > > We should do the same for pg_dump in -Fc mode. > > Did you meant -Fp mode. I think we are already setting stdout file to > binary mode if the output format is custom. Please refer to the > following code in parseArchiveFormat() and _allocAH() respectively > > static ArchiveFormat > parseArchiveFormat(const char *format, ArchiveMode *mode) > { > ............... > ............... > else if (pg_strcasecmp(format, "c") == 0) > archiveFormat = archCustom; > else if (pg_strcasecmp(format, "custom") == 0) > archiveFormat = archCustom; > > else if (pg_strcasecmp(format, "p") == 0) > archiveFormat = archNull; > else if (pg_strcasecmp(format, "plain") == 0) > archiveFormat = archNull; > ............... > ............... > } > > static ArchiveHandle * > _allocAH(const char *FileSpec, const ArchiveFormat fmt, > const int compression, bool dosync, ArchiveMode mode, > SetupWorkerPtrType setupWorkerPtr) > { > > ............... > ............... > #ifdef WIN32 > if (fmt != archNull && > (AH->fSpec == NULL || strcmp(AH->fSpec, "") == 0)) > { > if (mode == archModeWrite) > setmode(fileno(stdout), O_BINARY); > else > setmode(fileno(stdin), O_BINARY); > } > #endif > .................. > .................. > } > > Please confirm. > I also think it is the plain text mode. There is no problem with text mode file that contains the CR LF characters. Meanwhile, I have unit tested the patch submitted by Hari upthread on > postgresql v10 and it works fine. Below are the steps that i have > followed to test Hari's patch. > > Without patch: > ============== > C:\Users\ashu\postgresql\TMP\test\bin>.\pg_basebackup.exe -D - -F t -X > none > base.tar > NOTICE: WAL archiving is not enabled; you must ensure that all required > WAL seg > ments are copied through other means to complete the backup > > C:\Users\ashu\postgresql\TMP\test\bin>tar -xf base.tar > tar: Skipping to next header > tar: Exiting with failure status due to previous errors > > > With patch: > =========== > C:\Users\ashu\git-clone-postgresql\postgresql\TMP\ > test\bin>.\pg_basebackup.exe > -D - -F t -X none > base.tar > NOTICE: WAL archiving is not enabled; you must ensure that all required > WAL seg > ments are copied through other means to complete the backup > > C:\Users\ashu\postgresql\TMP\test\bin>cp base.tar ..\basebakup > > C:\Users\ashu\postgresql\TMP\test\basebakup>tar -xf base.tar > > C:\Users\ashu\postgresql\TMP\test\basebakup>ls > PG_VERSION pg_commit_ts pg_multixact pg_stat pg_wal > backup_label pg_dynshmem pg_notify pg_stat_tmp pg_xact > base pg_hba.conf pg_replslot pg_subtrans > postgresql.auto.conf > base.tar pg_ident.conf pg_serial pg_tblspc postgresql.conf > global pg_logical pg_snapshots pg_twophase tablespace_map > > Thanks for the tests to verify the patch. Regards, Hari Babu Fujitsu Australia
