Thread: pg_restore fails with a custom backup file
Hi, pg_restore faied by the following operations on Windows XP. $ createdb test $ pgbench -i -s 1000 test $ pg_dump -Fc test > out $ createdb restore $ pg_restore -d restore out pg_restore:[custom archiver] error during file seek: Invalid argument Win32 does not implement fseeko() and ftello(). So I think it limit to handle a 2GB file. Is this a specification? Regards, -- Yoshiyuki Asaba y-asaba@sraoss.co.jp
Hi. Asaba-san. From: "Yoshiyuki Asaba" > Win32 does not implement fseeko() and ftello(). So I think it limit to > handle a 2GB file. Is this a specification? Yes, Magnus-san suggested the problem. It is present TODO. The entire adjustment was still difficult though I had tried it. SetFilePointer might be able to be saved. However, I think it might be an attempt of 8.3... Regards, Hiroshi Saito
On Fri, Dec 15, 2006 at 12:57:50AM +0900, Hiroshi Saito wrote: > > >Win32 does not implement fseeko() and ftello(). So I think it limit to > >handle a 2GB file. Is this a specification? > > Yes, Magnus-san suggested the problem. It is present TODO. The entire > adjustment was still difficult though I had tried it. SetFilePointer might > be able to be saved. However, I think it might be an attempt of 8.3... I've been looking at a fix for this, and I think I have it. The solution looks to be to redefine off_t to 64-bit (the standard headers *always* define it as 32-bit, and there is no way to change that - at least not that I can find). I have the fix made for just bin/pg_dump for now (in pg_dump.h), and I'm testing that. (So far only on MSVC builds) A question though - is there any *gain* from using 64-bit offsets in the actual backend? The change could of course be done in port.h, but that will affect the whole backend (and require a few more functions than just fseeko/ftello to be redefined) which could have larger consequences. So - provided that this works after my test is completed, is the better place to do this for just pg_dump/pg_restore, or attempt to do it for the whole backend? //Magnus
Magnus Hagander wrote: > On Fri, Dec 15, 2006 at 12:57:50AM +0900, Hiroshi Saito wrote: > > > > >Win32 does not implement fseeko() and ftello(). So I think it limit to > > >handle a 2GB file. Is this a specification? > > > > Yes, Magnus-san suggested the problem. It is present TODO. The entire > > adjustment was still difficult though I had tried it. SetFilePointer might > > be able to be saved. However, I think it might be an attempt of 8.3... > > I've been looking at a fix for this, and I think I have it. The solution > looks to be to redefine off_t to 64-bit (the standard headers *always* > define it as 32-bit, and there is no way to change that - at least not > that I can find). > > I have the fix made for just bin/pg_dump for now (in pg_dump.h), and I'm > testing that. (So far only on MSVC builds) > > A question though - is there any *gain* from using 64-bit offsets in the > actual backend? The change could of course be done in port.h, but that No, not really. All files are kept < 1gig for the backend. We had code for that from Berkeley, so we have just kept it. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Hi. Oh, your great trust confidence.:-) >> I have the fix made for just bin/pg_dump for now (in pg_dump.h), and I'm >> testing that. (So far only on MSVC builds) However, MinGW+gcc be able to be saved? I was wishing it.... Regards, Hiroshi Saito
On Mon, Dec 18, 2006 at 09:50:12AM -0500, Bruce Momjian wrote: > > > Yes, Magnus-san suggested the problem. It is present TODO. The entire > > > adjustment was still difficult though I had tried it. SetFilePointer might > > > be able to be saved. However, I think it might be an attempt of 8.3... > > > > I've been looking at a fix for this, and I think I have it. The solution > > looks to be to redefine off_t to 64-bit (the standard headers *always* > > define it as 32-bit, and there is no way to change that - at least not > > that I can find). > > > > I have the fix made for just bin/pg_dump for now (in pg_dump.h), and I'm > > testing that. (So far only on MSVC builds) > > > > A question though - is there any *gain* from using 64-bit offsets in the > > actual backend? The change could of course be done in port.h, but that > > No, not really. All files are kept < 1gig for the backend. We had code > for that from Berkeley, so we have just kept it. Ok, based on this, here's a patch that *appears* to fix the problem on Win32. I tried the suggested repro to dump a pgbench database of appropriate size, and it does restore properly and give me the proper amount of rows in all the tables. But that's all I've tested so far. Also, it compiles fine on MSVC. I still haven't managed to get the MingW build environment working properly on Win64 even for building Win32 apps, so I haven't been able to build it on MingW yet. It *should* work since it's all standard functions, but might require further hacks. I'll try to get around to that later, and Dave has promised to give it a compile-try as well, but if someone wants to test that, please do ;) So, does it seem reasonable, and the right place to stick it? Oh, and please do *not* apply until someone confirms it works on mingw! //Magnus
Attachment
Bruce Momjian <bruce@momjian.us> writes:
> Magnus Hagander wrote:
>> A question though - is there any *gain* from using 64-bit offsets in the
>> actual backend? The change could of course be done in port.h, but that
> No, not really. All files are kept < 1gig for the backend.
Not so: consider a backend COPY reading or writing a multi-gig table.
This will fail if the platform hasn't got largefile support.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Magnus Hagander wrote: > >> A question though - is there any *gain* from using 64-bit offsets in the > >> actual backend? The change could of course be done in port.h, but that > > > No, not really. All files are kept < 1gig for the backend. > > Not so: consider a backend COPY reading or writing a multi-gig table. > This will fail if the platform hasn't got largefile support. Good point --- but do we do any seeks in COPY files? I don't think so, so I don't see how we would benefit from large file support there. Certainly people are dumping >2 gig files. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> Not so: consider a backend COPY reading or writing a multi-gig table.
> Good point --- but do we do any seeks in COPY files? I don't think so,
True, so if the problem is limited to whether we can seek or not, then
we don't need to fix the backend. Magnus said something about other
issues though?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> Not so: consider a backend COPY reading or writing a multi-gig table. > > > Good point --- but do we do any seeks in COPY files? I don't think so, > > True, so if the problem is limited to whether we can seek or not, then > we don't need to fix the backend. Magnus said something about other > issues though? Yes, MinGW. I think he is researching that. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Magnus Hagander wrote:
> Index: src/bin/pg_dump/pg_dump.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dump.h,v
> retrieving revision 1.130
> diff -c -r1.130 pg_dump.h
> *** src/bin/pg_dump/pg_dump.h 9 Oct 2006 23:36:59 -0000 1.130
> --- src/bin/pg_dump/pg_dump.h 18 Dec 2006 14:33:16 -0000
> ***************
> *** 16,21 ****
> --- 16,33 ----
>
> #include "postgres_fe.h"
>
> + #ifdef WIN32
> + /*
> + * WIN32 does not provide a 64-bit off_t, but it does provide functions operating
> + * with 64-bit offsets. Redefine off_t to what's always a 64-bit int, and redefine
> + * the functions that accept off_t to be the 64-bit only ones.
> + */
> + #define off_t __int64
> + #undef fseeko
> + #define fseeko(stream, offset, origin) _fseeki64(stream, offset, origin)
> + #undef ftello
> + #define ftello(stream) _ftelli64(stream)
> + #endif
>
> /*
> * pg_dump uses two different mechanisms for identifying database objects:
>
This patch appears to be broken on my MinGW setup:
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g
-DFRONTEND -I../../../src/interfaces/libpq -I../.
./../src/include -I./src/include/port/win32 -DEXEC_BACKEND
-I/c/tcl/include "-I../../../src/include/port/win32" -c -o common.o
common.c -MMD -MP -MF .deps/common.Po
In file included from
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/zconf.h:289, from
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/zlib.h:34, from pg_backup_archiver.h:44,
from common.c:20:
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/unistd.h:23:
error: conflicting types for 'chsize'
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/io.h:271:
error: previous declaration of 'chsize' was here
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/unistd.h:23:
error: conflicting types for 'chsize'
c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/io.h:271:
error: previous declaration of 'chsize' was here
I suspect we might need to create a pg_off_t type or some such gadget.
Bleah.
But it does need to be fixed.
cheers
andrew
Andrew Dunstan wrote: > Magnus Hagander wrote: >> Index: src/bin/pg_dump/pg_dump.h >> =================================================================== >> RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dump.h,v >> retrieving revision 1.130 >> diff -c -r1.130 pg_dump.h >> *** src/bin/pg_dump/pg_dump.h 9 Oct 2006 23:36:59 -0000 1.130 >> --- src/bin/pg_dump/pg_dump.h 18 Dec 2006 14:33:16 -0000 >> *************** >> *** 16,21 **** >> --- 16,33 ---- >> #include "postgres_fe.h" >> + #ifdef WIN32 >> + /* >> + * WIN32 does not provide a 64-bit off_t, but it does provide >> functions operating >> + * with 64-bit offsets. Redefine off_t to what's always a 64-bit >> int, and redefine >> + * the functions that accept off_t to be the 64-bit only ones. >> + */ >> + #define off_t __int64 >> + #undef fseeko >> + #define fseeko(stream, offset, origin) _fseeki64(stream, offset, >> origin) >> + #undef ftello >> + #define ftello(stream) _ftelli64(stream) >> + #endif >> /* >> * pg_dump uses two different mechanisms for identifying database >> objects: >> > > This patch appears to be broken on my MinGW setup: > > gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline > -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g > -DFRONTEND -I../../../src/interfaces/libpq -I../. > ./../src/include -I./src/include/port/win32 -DEXEC_BACKEND > -I/c/tcl/include "-I../../../src/include/port/win32" -c -o common.o > common.c -MMD -MP -MF .deps/common.Po > In file included from > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/zconf.h:289, > from > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/zlib.h:34, > from pg_backup_archiver.h:44, > from common.c:20: > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/unistd.h:23: > error: conflicting types for 'chsize' > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/io.h:271: > error: previous declaration of 'chsize' was here > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/unistd.h:23: > error: conflicting types for 'chsize' > c:/mingw/bin/../lib/gcc/mingw32/3.4.2/../../../../include/io.h:271: > error: previous declaration of 'chsize' was here > > > I suspect we might need to create a pg_off_t type or some such gadget. > > Bleah. > > But it does need to be fixed. Bummer. That might be what's needed, but I'm going to at least try to find some neater way first. I wonder why it didn't happen on MSVC... //Magnus
Magnus Hagander <magnus@hagander.net> writes:
> Andrew Dunstan wrote:
>> I suspect we might need to create a pg_off_t type or some such gadget.
>> Bleah.
> Bummer. That might be what's needed, but I'm going to at least try to
> find some neater way first. I wonder why it didn't happen on MSVC...
Seems like this can only work if the extra #define's appear *after* all
system header files, which might or might not be practical --- but
you'll have as much trouble with the function #define's as the typedef
one if you include headers defining the functions after the macros appear.
regards, tom lane
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Andrew Dunstan wrote: >>> I suspect we might need to create a pg_off_t type or some such gadget. >>> Bleah. > >> Bummer. That might be what's needed, but I'm going to at least try to >> find some neater way first. I wonder why it didn't happen on MSVC... > > Seems like this can only work if the extra #define's appear *after* all > system header files, which might or might not be practical --- but > you'll have as much trouble with the function #define's as the typedef > one if you include headers defining the functions after the macros appear. The functions aren't really a problem I think - they don't exist on win32. They're #defined from fseeko() to fseek() etc in port.h. //Magnus
>> I suspect we might need to create a pg_off_t type or some such gadget. >> >> Bleah. >> >> But it does need to be fixed. > > Bummer. That might be what's needed, but I'm going to at least try to > find some neater way first. I wonder why it didn't happen on MSVC... Hmm. This was even worse than I thought :-( I got it building most of the way by following Andrews suggestion and greating a pgoff_t, just to check it out. That done, it seems that mingw doesn't include these 64-bit functions in their import library *at all*. That gives us basically two options that I can see, to proceed: 1) Set up pg_dump* to dynamically load these functions from msvcrt.dll at startup. This will require a different codepath from the MSVC build of course, since Microsoft have been shipping these functions in their libraries since NT4. Should work, but nor particularly pretty. 2) Just say that the mingw compiled versions of pg_dump* can't deal with 2Gb+ files. IIRC, we've built pg_dump with both the "new vc build system" on VS2005 and with the "old win32.mak style build system" with VC++ 6.0, so if we're comfortable enough with that we could just ship binaries built with VC++ for those utilities (even if we don't go to shipping completely MSVC built binaries for 8.3). Thoughts on these options? //Magnus
Magnus Hagander wrote: >>> I suspect we might need to create a pg_off_t type or some such gadget. >>> >>> Bleah. >>> >>> But it does need to be fixed. >>> >> Bummer. That might be what's needed, but I'm going to at least try to >> find some neater way first. I wonder why it didn't happen on MSVC... >> > > Hmm. This was even worse than I thought :-( > > I got it building most of the way by following Andrews suggestion and > greating a pgoff_t, just to check it out. That done, it seems that mingw > doesn't include these 64-bit functions in their import library *at all*. > That gives us basically two options that I can see, to proceed: > > 1) Set up pg_dump* to dynamically load these functions from msvcrt.dll > at startup. This will require a different codepath from the MSVC build > of course, since Microsoft have been shipping these functions in their > libraries since NT4. Should work, but nor particularly pretty. > > 2) Just say that the mingw compiled versions of pg_dump* can't deal with > 2Gb+ files. IIRC, we've built pg_dump with both the "new vc build > system" on VS2005 and with the "old win32.mak style build system" with > VC++ 6.0, so if we're comfortable enough with that we could just ship > binaries built with VC++ for those utilities (even if we don't go to > shipping completely MSVC built binaries for 8.3). > > > Thoughts on these options? > > //Magnus > > Triple bleah. It is not acceptable to say that our only open source tool chain can't build fundamentally required functionality. A little googling on "mingw ftello64" came up with this link, which looked somewhat promising: http://www.nabble.com/RE:-ftello64-returning-wrong-values-p703470.html cheers andrew
>> Hmm. This was even worse than I thought :-( >> >> I got it building most of the way by following Andrews suggestion and >> greating a pgoff_t, just to check it out. That done, it seems that mingw >> doesn't include these 64-bit functions in their import library *at all*. >> That gives us basically two options that I can see, to proceed: >> >> 1) Set up pg_dump* to dynamically load these functions from msvcrt.dll >> at startup. This will require a different codepath from the MSVC build >> of course, since Microsoft have been shipping these functions in their >> libraries since NT4. Should work, but nor particularly pretty. >> >> 2) Just say that the mingw compiled versions of pg_dump* can't deal with >> 2Gb+ files. IIRC, we've built pg_dump with both the "new vc build >> system" on VS2005 and with the "old win32.mak style build system" with >> VC++ 6.0, so if we're comfortable enough with that we could just ship >> binaries built with VC++ for those utilities (even if we don't go to >> shipping completely MSVC built binaries for 8.3). >> >> >> Thoughts on these options? >> >> //Magnus >> >> > > Triple bleah. It is not acceptable to say that our only open source tool > chain can't build fundamentally required functionality. > > A little googling on "mingw ftello64" came up with this link, which > looked somewhat promising: > > http://www.nabble.com/RE:-ftello64-returning-wrong-values-p703470.html What the heck. So it seems mingw went ahead and implemented their own ftello64 function *without* using the 64-bit functions as provided by the standard libraires. Argh. There's also fseeko64(). These are of course mingw only, so we'll need one codepath for mingw and one for MSVC, but it does at least seem doable. We're still going to have to change from off_t to pgoff_t or similar, since MSVC will need int64 and mingw will need off64_t.. Right? I'll try to take a look at this sometime the next couple of days (out of time for today) unless beaten to it. //Magnus
Magnus Hagander wrote: >>> Hmm. This was even worse than I thought :-( >>> >>> I got it building most of the way by following Andrews suggestion and >>> greating a pgoff_t, just to check it out. That done, it seems that mingw >>> doesn't include these 64-bit functions in their import library *at all*. >>> That gives us basically two options that I can see, to proceed: >>> >>> 1) Set up pg_dump* to dynamically load these functions from msvcrt.dll >>> at startup. This will require a different codepath from the MSVC build >>> of course, since Microsoft have been shipping these functions in their >>> libraries since NT4. Should work, but nor particularly pretty. >>> >>> 2) Just say that the mingw compiled versions of pg_dump* can't deal with >>> 2Gb+ files. IIRC, we've built pg_dump with both the "new vc build >>> system" on VS2005 and with the "old win32.mak style build system" with >>> VC++ 6.0, so if we're comfortable enough with that we could just ship >>> binaries built with VC++ for those utilities (even if we don't go to >>> shipping completely MSVC built binaries for 8.3). >>> >>> >>> Thoughts on these options? >>> >>> //Magnus >>> >>> >> Triple bleah. It is not acceptable to say that our only open source tool >> chain can't build fundamentally required functionality. >> >> A little googling on "mingw ftello64" came up with this link, which >> looked somewhat promising: >> >> http://www.nabble.com/RE:-ftello64-returning-wrong-values-p703470.html > > What the heck. So it seems mingw went ahead and implemented their own > ftello64 function *without* using the 64-bit functions as provided by > the standard libraires. Argh. There's also fseeko64(). These are of > course mingw only, so we'll need one codepath for mingw and one for > MSVC, but it does at least seem doable. > > We're still going to have to change from off_t to pgoff_t or similar, > since MSVC will need int64 and mingw will need off64_t.. Right? > > I'll try to take a look at this sometime the next couple of days (out of > time for today) unless beaten to it. > Actually, there's another option that Hiroshi mentioned off-list, that I forgot. We can implement the Microsoft functions _fseeki64() and _ftelli64() ourselves, based on win32 API functions. There are examples available for this. Not sure which is the cleanest method, too late for more thinking ;-) //Magnus
Magnus Hagander wrote: > > I'll try to take a look at this sometime the next couple of days (out of > > time for today) unless beaten to it. > > > > Actually, there's another option that Hiroshi mentioned off-list, that I > forgot. > > We can implement the Microsoft functions _fseeki64() and _ftelli64() > ourselves, based on win32 API functions. There are examples available > for this. > > Not sure which is the cleanest method, too late for more thinking ;-) See src/port/fseeko.c for an example for NetBSD and BSD/OS. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Magnus Hagander wrote: > Also, it compiles fine on MSVC. I still haven't managed to get the MingW > build environment working properly on Win64 even for building Win32 > apps, so I haven't been able to build it on MingW yet. It *should* work > since it's all standard functions, but might require further hacks. I'll > try to get around to that later, and Dave has promised to give it a > compile-try as well, but if someone wants to test that, please do ;) :-( gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing pg_dump.o common.o pg_dump_sort.o pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o pg_backup_files.o pg_backup_null.o pg_backup_tar.o dumputils.o win32ver.o ../../../src/backend/parser/keywords.o -L../../../src/port -lpgport -L../../../src/interfaces/libpq -lpq -L../../../src/port -Wl,--allow-multiple-definition -lpgport -lintl -lssleay32 -leay32 -lcomerr32 -lkrb5_32 -lz -lm -lws2_32 -lshfolder -o pg_dump.exe pg_backup_archiver.o(.text+0x2017):pg_backup_archiver.c: undefined reference to `_fseeki64' pg_backup_archiver.o(.text+0x3dac):pg_backup_archiver.c: undefined reference to `_fseeki64' pg_backup_custom.o(.text+0x773):pg_backup_custom.c: undefined reference to `_fseeki64' pg_backup_custom.o(.text+0xaaa):pg_backup_custom.c: undefined reference to `_ftelli64' pg_backup_custom.o(.text+0xed2):pg_backup_custom.c: undefined reference to `_ftelli64' pg_backup_custom.o(.text+0xf21):pg_backup_custom.c: undefined reference to `_fseeki64' pg_backup_tar.o(.text+0x845):pg_backup_tar.c: undefined reference to `_ftelli64' pg_backup_tar.o(.text+0x10f9):pg_backup_tar.c: undefined reference to `_fseeki64' pg_backup_tar.o(.text+0x1107):pg_backup_tar.c: undefined reference to `_ftelli64' pg_backup_tar.o(.text+0x1162):pg_backup_tar.c: undefined reference to `_fseeki64' collect2: ld returned 1 exit status make[3]: *** [pg_dump] Error 1 make[3]: Leaving directory `/cvs/pgsql/src/bin/pg_dump' Regards, Dave.
> > I suspect we might need to create a pg_off_t type or some > such gadget. > > > > Bleah. > > > > But it does need to be fixed. > > Bummer. That might be what's needed, but I'm going to at least try to > find some neater way first. I wonder why it didn't happen on MSVC... I don't see how the error relates, but _fseeki64 and _ftelli64 is only in msvcr80.dll and newer, not below. MinGW has fseeko64 and ftello64 with off64_t. Andreas
Hi, From: "Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> Subject: Re: [HACKERS] pg_restore fails with a custom backup file Date: Fri, 15 Dec 2006 00:57:50 +0900 > > Win32 does not implement fseeko() and ftello(). So I think it limit to > > handle a 2GB file. Is this a specification? > > Yes, Magnus-san suggested the problem. It is present TODO. The entire > adjustment was still difficult though I had tried it. SetFilePointer might be > able to be saved. However, I think it might be an attempt of 8.3... Is it able to use fsetpos()/fgetpos() instead of ftell()/fseek()? fpos_t is a 8byte type. I tested pg_dump/pg_restore with the attached patch. -- Yoshiyuki Asaba y-asaba@sraoss.co.jp Index: src/include/c.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/include/c.h,v retrieving revision 1.214 diff -c -r1.214 c.h *** src/include/c.h 4 Oct 2006 00:30:06 -0000 1.214 --- src/include/c.h 19 Dec 2006 12:52:05 -0000 *************** *** 74,79 **** --- 74,82 ---- #include <strings.h> #endif #include <sys/types.h> + #ifdef WIN32 + #define off_t fpos_t + #endif #include <errno.h> #if defined(WIN32) || defined(__CYGWIN__) Index: src/include/port.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/include/port.h,v retrieving revision 1.106 diff -c -r1.106 port.h *** src/include/port.h 28 Nov 2006 01:12:33 -0000 1.106 --- src/include/port.h 19 Dec 2006 12:52:05 -0000 *************** *** 307,313 **** extern char *crypt(const char *key, const char *setting); #endif ! #if defined(bsdi) || defined(netbsd) extern int fseeko(FILE *stream, off_t offset, int whence); extern off_t ftello(FILE*stream); #endif --- 307,313 ---- extern char *crypt(const char *key, const char *setting); #endif ! #if defined(bsdi) || defined(netbsd) || defined(WIN32) extern int fseeko(FILE *stream, off_t offset, int whence); externoff_t ftello(FILE *stream); #endif Index: src/include/port/win32.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/include/port/win32.h,v retrieving revision 1.63 diff -c -r1.63 win32.h *** src/include/port/win32.h 19 Oct 2006 20:03:08 -0000 1.63 --- src/include/port/win32.h 19 Dec 2006 12:52:05 -0000 *************** *** 20,25 **** --- 20,27 ---- #include <sys/utime.h> /* for non-unicode version */ #undef near + #define HAVE_FSEEKO + /* Must be here to avoid conflicting with prototype in windows.h */ #define mkdir(a,b) mkdir(a) Index: src/port/fseeko.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/port/fseeko.c,v retrieving revision 1.20 diff -c -r1.20 fseeko.c *** src/port/fseeko.c 5 Mar 2006 15:59:10 -0000 1.20 --- src/port/fseeko.c 19 Dec 2006 12:52:06 -0000 *************** *** 17,23 **** * We have to use the native defines here because configure hasn't * completed yet. */ ! #if defined(__bsdi__) || defined(__NetBSD__) #include "c.h" --- 17,23 ---- * We have to use the native defines here because configure hasn't * completed yet. */ ! #if defined(__bsdi__) || defined(__NetBSD__) || defined(WIN32) #include "c.h"
On Tue, Dec 19, 2006 at 09:59:05PM +0900, Yoshiyuki Asaba wrote: > Hi, > > > > Win32 does not implement fseeko() and ftello(). So I think it limit to > > > handle a 2GB file. Is this a specification? > > > > Yes, Magnus-san suggested the problem. It is present TODO. The entire > > adjustment was still difficult though I had tried it. SetFilePointer might be > > able to be saved. However, I think it might be an attempt of 8.3... > > Is it able to use fsetpos()/fgetpos() instead of ftell()/fseek()? > fpos_t is a 8byte type. I tested pg_dump/pg_restore with the attached > patch. Hmm. Yeah, that should work in principle. However, did you test the actual backend after that change? Given where you change the define of off_t, that would affect every call in the backend that uses off_t, and it just seems very strange that you could get away with that without touching anything else? (If we're lucky, but I wouldn't count on it - there ought to be other functions in libc that we call that takes off_t..) //Magnus
Magnus Hagander wrote: > On Tue, Dec 19, 2006 at 09:59:05PM +0900, Yoshiyuki Asaba wrote: > >> Hi, >> >> >>>> Win32 does not implement fseeko() and ftello(). So I think it limit to >>>> handle a 2GB file. Is this a specification? >>>> >>> Yes, Magnus-san suggested the problem. It is present TODO. The entire >>> adjustment was still difficult though I had tried it. SetFilePointer might be >>> able to be saved. However, I think it might be an attempt of 8.3... >>> >> Is it able to use fsetpos()/fgetpos() instead of ftell()/fseek()? >> fpos_t is a 8byte type. I tested pg_dump/pg_restore with the attached >> patch. >> > > Hmm. Yeah, that should work in principle. > > However, did you test the actual backend after that change? Given where you > change the define of off_t, that would affect every call in the backend > that uses off_t, and it just seems very strange that you could get away > with that without touching anything else? (If we're lucky, but I > wouldn't count on it - there ought to be other functions in libc that we > call that takes off_t..) > I'd feel much happier if we could just patch pg_dump, since this is the only place we know of that we need to do large file seek/tell operations. Did you see this from Andreas? > MinGW has fseeko64 and ftello64 with off64_t. > Maybe we need separate macros for MSVC and MinGW. Given the other interactions we might need to push those deep into the C files after all the system headers. Maybe create pg_dump_fseek.h and put them in there and then #include that very late. cheers andrew
> >However, did you test the actual backend after that change? Given where you > >change the define of off_t, that would affect every call in the backend > >that uses off_t, and it just seems very strange that you could get away > >with that without touching anything else? (If we're lucky, but I > >wouldn't count on it - there ought to be other functions in libc that we > >call that takes off_t..) > > > > I'd feel much happier if we could just patch pg_dump, since this is the > only place we know of that we need to do large file seek/tell operations. My thoughts exactly. > Did you see this from Andreas? > > >MinGW has fseeko64 and ftello64 with off64_t. > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > interactions we might need to push those deep into the C files after all > the system headers. Maybe create pg_dump_fseek.h and put them in there > and then #include that very late. We need different macrosand possibly functions, yes. I think I got enough patched at home last night to get it working with this, I was just too focused on one set of macros at the time. It's not enough to include them very late - because off_t is used in the shared datastructures in pg_dump/etc. It is possible to localise it to the pg_dump binaries, though, given some header redirection *and* given that we change all those off_t to pgoff_t (or similar). I couldn't find a way to do it without changing the off_t define. I'll try to take a look at merging these two efforts (again unless beaten to it, have to do some of that dreaded christmas shopping as well...) //Magnus
> Did you see this from Andreas? > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > Maybe we need separate macros for MSVC and MinGW. Given the other You mean something quick and dirty like this ? That would work. Andreas
Attachment
On Tue, Dec 19, 2006 at 04:25:18PM +0100, Zeugswetter Andreas ADI SD wrote: > > > Did you see this from Andreas? > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > You mean something quick and dirty like this ? That would work. Yes, except does that actually work? If so you found the place in the headers to stick it without breaking things that I couldn't find ;-) I got compile warnings (note, warnings, not errors, for some reason, but very significant) about sending 64-bit ints to API functions that were 32-bits and such without creating a separate define for off_t. Could very well be that I was too tired and too focused on the websearch stuff when I tried it though :-) //Magnus
> > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > You mean something quick and dirty like this ? That would work. > > Yes, except does that actually work? If so you found the place in the > headers to stick it without breaking things that I couldn't find ;-) Compiles clean without warnings on MinGW, but not tested, sorry also no time. Andreas
Magnus Hagander wrote: > We need different macrosand possibly functions, yes. > I think I got enough patched at home last night to get it working with > this, I was just too focused on one set of macros at the time. It's not > enough to include them very late - because off_t is used in the shared > datastructures in pg_dump/etc. It is possible to localise it to the > pg_dump binaries, though, given some header redirection *and* given that > we change all those off_t to pgoff_t (or similar). I couldn't find a way > to do it without changing the off_t define. > > I'll try to take a look at merging these two efforts (again unless > beaten to it, have to do some of that dreaded christmas shopping as > well...) > > > What is needed to test this? Just a custom dump file with a member > 2^31 bytes? cheers andrew
Andrew Dunstan wrote: > Magnus Hagander wrote: >> We need different macrosand possibly functions, yes. >> I think I got enough patched at home last night to get it working with >> this, I was just too focused on one set of macros at the time. It's not >> enough to include them very late - because off_t is used in the shared >> datastructures in pg_dump/etc. It is possible to localise it to the >> pg_dump binaries, though, given some header redirection *and* given that >> we change all those off_t to pgoff_t (or similar). I couldn't find a way >> to do it without changing the off_t define. >> >> I'll try to take a look at merging these two efforts (again unless >> beaten to it, have to do some of that dreaded christmas shopping as >> well...) >> >> >> > > What is needed to test this? Just a custom dump file with a member > > 2^31 bytes? Yeah, I believe so. It backs it up fine, but it won't restore it properly. You can create such a database with the pgbench tool and the correct scaling factor, per the original mail in this thread. //Magnus
Hi Asaba-san. From: "Yoshiyuki Asaba" > Is it able to use fsetpos()/fgetpos() instead of ftell()/fseek()? > fpos_t is a 8byte type. I tested pg_dump/pg_restore with the attached > patch. I'm sorry the response ..slowly...my machine reacts for the reasons of poverty late. Last night.. I was actually looking at your proposal. I was trying correcting by another point. It also saw one solution. I think content that your proposal also contributes enough. However, I think there is worry that Magnus-san means, too. There are your proposal, a proposal of Andreas-san, and my proposal now. Surely, I think that it offers Magnus-san again after arranging these by a good solution.:-) P.S) I understand, your a lot time be spent on this work. patch-test-check-patch-test-check.... Oh..., It takes large amount of time to big data... Anyway, thanks!!:-) Regards, Hiroshi Saito
On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: > > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > > > You mean something quick and dirty like this ? That would work. > > > > Yes, except does that actually work? If so you found the place in the > > headers to stick it without breaking things that I couldn't find ;-) > > Compiles clean without warnings on MinGW, but not tested, sorry also no > time. Does not compile on my MinGW - errors in the system headers (unistd.h, io.h) due to changing the argument format for chsize(). The change of off_t propagated into parts of the system headers, thus chaos was ensured. I still think we need to use a pgoff_t. Will look at combining these two approaches. //Magnus
Where are we on this? --------------------------------------------------------------------------- Magnus Hagander wrote: > On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: > > > > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > > > > > You mean something quick and dirty like this ? That would work. > > > > > > Yes, except does that actually work? If so you found the place in the > > > headers to stick it without breaking things that I couldn't find ;-) > > > > Compiles clean without warnings on MinGW, but not tested, sorry also no > > time. > > Does not compile on my MinGW - errors in the system headers (unistd.h, > io.h) due to changing the argument format for chsize(). The change of > off_t propagated into parts of the system headers, thus chaos was > ensured. > > I still think we need to use a pgoff_t. Will look at combining these two > approaches. > > //Magnus > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Still sitting on my TODO. I have a working solution for MSVC, but it didn't run on MingW. Andreas had a working solution on his MingW, but it didn't work on my MingW. I need to merge them together for something that works on all three. I hope to have this done for 8.3, and possibly a 8.2.x, but will most likely no thave time to do it before the next 8.2.x to come out. But perhaps the one after that. //Magnus On Wed, Jan 31, 2007 at 11:41:09PM -0500, Bruce Momjian wrote: > > Where are we on this? > > --------------------------------------------------------------------------- > > Magnus Hagander wrote: > > On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: > > > > > > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > > > > > > > You mean something quick and dirty like this ? That would work. > > > > > > > > Yes, except does that actually work? If so you found the place in the > > > > headers to stick it without breaking things that I couldn't find ;-) > > > > > > Compiles clean without warnings on MinGW, but not tested, sorry also no > > > time. > > > > Does not compile on my MinGW - errors in the system headers (unistd.h, > > io.h) due to changing the argument format for chsize(). The change of > > off_t propagated into parts of the system headers, thus chaos was > > ensured. > > > > I still think we need to use a pgoff_t. Will look at combining these two > > approaches. > > > > //Magnus > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 6: explain analyze is your friend > > -- > Bruce Momjian bruce@momjian.us > EnterpriseDB http://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. +
Thread URL added to TODO item:
o Add long file support for binary pg_dump output
---------------------------------------------------------------------------
Magnus Hagander wrote:
> On Fri, Dec 15, 2006 at 12:57:50AM +0900, Hiroshi Saito wrote:
> >
> > >Win32 does not implement fseeko() and ftello(). So I think it limit to
> > >handle a 2GB file. Is this a specification?
> >
> > Yes, Magnus-san suggested the problem. It is present TODO. The entire
> > adjustment was still difficult though I had tried it. SetFilePointer might
> > be able to be saved. However, I think it might be an attempt of 8.3...
>
> I've been looking at a fix for this, and I think I have it. The solution
> looks to be to redefine off_t to 64-bit (the standard headers *always*
> define it as 32-bit, and there is no way to change that - at least not
> that I can find).
>
> I have the fix made for just bin/pg_dump for now (in pg_dump.h), and I'm
> testing that. (So far only on MSVC builds)
>
> A question though - is there any *gain* from using 64-bit offsets in the
> actual backend? The change could of course be done in port.h, but that
> will affect the whole backend (and require a few more functions than
> just fseeko/ftello to be redefined) which could have larger
> consequences.
>
> So - provided that this works after my test is completed, is the better
> place to do this for just pg_dump/pg_restore, or attempt to do it for
> the whole backend?
>
> //Magnus
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
-- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Fri, Dec 29, 2006 at 05:30:48PM +0100, Magnus Hagander wrote: > On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: > > > > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > > > > > You mean something quick and dirty like this ? That would work. > > > > > > Yes, except does that actually work? If so you found the place in the > > > headers to stick it without breaking things that I couldn't find ;-) > > > > Compiles clean without warnings on MinGW, but not tested, sorry also no > > time. > > Does not compile on my MinGW - errors in the system headers (unistd.h, > io.h) due to changing the argument format for chsize(). The change of > off_t propagated into parts of the system headers, thus chaos was > ensured. > > I still think we need to use a pgoff_t. Will look at combining these two > approaches. Here's a patch that tries this. *needs more testing*. But built with this patch, I can dump and restore a table at the end of a 10gb database without errors. Does the method/patch seem reasonable? Anybody else who can run a couple of tests on it? //Magnus
Attachment
Hi Magnus-san. Great!! Although not tested yet, I seem to equip it with the tolerance to 32GB.? P.S) In Japan, there is a user who is employing 300GB of database on Windows2003. I have received some problems other than this. however, this user does not permit public presentation of the information.... Then, I have asked that the information is exhibited. ..There is no still good reply. Regards, Hiroshi Saito > On Fri, Dec 29, 2006 at 05:30:48PM +0100, Magnus Hagander wrote: >> On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: >> > >> > > > > > MinGW has fseeko64 and ftello64 with off64_t. >> > > > > > >> > > > > >> > > > > Maybe we need separate macros for MSVC and MinGW. Given the other >> > > > >> > > > You mean something quick and dirty like this ? That would work. >> > > >> > > Yes, except does that actually work? If so you found the place in the >> > > headers to stick it without breaking things that I couldn't find ;-) >> > >> > Compiles clean without warnings on MinGW, but not tested, sorry also no >> > time. >> >> Does not compile on my MinGW - errors in the system headers (unistd.h, >> io.h) due to changing the argument format for chsize(). The change of >> off_t propagated into parts of the system headers, thus chaos was >> ensured. >> >> I still think we need to use a pgoff_t. Will look at combining these two >> approaches. > > Here's a patch that tries this. > *needs more testing*. But built with this patch, I can dump and > restore a table at the end of a 10gb database without errors. > > Does the method/patch seem reasonable? Anybody else who can run a couple > of tests on it? > > //Magnus >
Hi, From: Magnus Hagander <magnus@hagander.net> Subject: Re: [HACKERS] pg_restore fails with a custom backup file Date: Thu, 15 Feb 2007 17:38:59 +0100 > On Fri, Dec 29, 2006 at 05:30:48PM +0100, Magnus Hagander wrote: > > On Tue, Dec 19, 2006 at 04:58:22PM +0100, Zeugswetter Andreas ADI SD wrote: > > > > > > > > > > MinGW has fseeko64 and ftello64 with off64_t. > > > > > > > > > > > > > > > > > > > Maybe we need separate macros for MSVC and MinGW. Given the other > > > > > > > > > > You mean something quick and dirty like this ? That would work. > > > > > > > > Yes, except does that actually work? If so you found the place in the > > > > headers to stick it without breaking things that I couldn't find ;-) > > > > > > Compiles clean without warnings on MinGW, but not tested, sorry also no > > > time. > > > > Does not compile on my MinGW - errors in the system headers (unistd.h, > > io.h) due to changing the argument format for chsize(). The change of > > off_t propagated into parts of the system headers, thus chaos was > > ensured. > > > > I still think we need to use a pgoff_t. Will look at combining these two > > approaches. > > Here's a patch that tries this. > *needs more testing*. But built with this patch, I can dump and > restore a table at the end of a 10gb database without errors. I tried the attached patch. But I got the following error. pg_backup_archiver.o(.text+0x1fa4): In function `allocAH': C:/msys/1.0/home/y-asaba/postgresql-8.2.3-patch/src/bin/pg_dump/pg_backup_archiver.c:1580: undefined reference to `fseeko64' ... make[3]: *** [pg_dump] Error 1 $ uname -sr MINGW32_NT-5.1 1.0.10(0.46/3/2) Is MINGW version too old? -- Yoshiyuki Asaba y-asaba@sraoss.co.jp
On Fri, Feb 16, 2007 at 02:09:41PM +0900, Yoshiyuki Asaba wrote: > > > Does not compile on my MinGW - errors in the system headers (unistd.h, > > > io.h) due to changing the argument format for chsize(). The change of > > > off_t propagated into parts of the system headers, thus chaos was > > > ensured. > > > > > > I still think we need to use a pgoff_t. Will look at combining these two > > > approaches. > > > > Here's a patch that tries this. > > *needs more testing*. But built with this patch, I can dump and > > restore a table at the end of a 10gb database without errors. > > I tried the attached patch. But I got the following error. > > pg_backup_archiver.o(.text+0x1fa4): In function `allocAH': > C:/msys/1.0/home/y-asaba/postgresql-8.2.3-patch/src/bin/pg_dump/pg_backup_archiver.c:1580: undefined reference to `fseeko64' > ... > make[3]: *** [pg_dump] Error 1 > > $ uname -sr > MINGW32_NT-5.1 1.0.10(0.46/3/2) > > Is MINGW version too old? I think so. It seems this was added in version 1.24 of stdio.h in mingw (http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/mingw/include/stdio.h?cvsroot=src). Could you try upgrading mingw and see if that helps? Or possibly instlaling side-by-side a different version (if they even allow that)? //Magnus
From: Magnus Hagander <magnus@hagander.net> Subject: Re: [HACKERS] pg_restore fails with a custom backup file Date: Fri, 16 Feb 2007 10:13:35 +0100 > On Fri, Feb 16, 2007 at 02:09:41PM +0900, Yoshiyuki Asaba wrote: > > > > > Does not compile on my MinGW - errors in the system headers (unistd.h, > > > > io.h) due to changing the argument format for chsize(). The change of > > > > off_t propagated into parts of the system headers, thus chaos was > > > > ensured. > > > > > > > > I still think we need to use a pgoff_t. Will look at combining these two > > > > approaches. > > > > > > Here's a patch that tries this. > > > *needs more testing*. But built with this patch, I can dump and > > > restore a table at the end of a 10gb database without errors. > > > > I tried the attached patch. But I got the following error. > > > > pg_backup_archiver.o(.text+0x1fa4): In function `allocAH': > > C:/msys/1.0/home/y-asaba/postgresql-8.2.3-patch/src/bin/pg_dump/pg_backup_archiver.c:1580: undefined reference to `fseeko64' > > ... > > make[3]: *** [pg_dump] Error 1 > > > > $ uname -sr > > MINGW32_NT-5.1 1.0.10(0.46/3/2) > > > > Is MINGW version too old? > > I think so. It seems this was added in version 1.24 of stdio.h in mingw > (http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/mingw/include/stdio.h?cvsroot=src). > Could you try upgrading mingw and see if that helps? Or possibly > instlaling side-by-side a different version (if they even allow that)? OK. I have upgraded mingw and tried to compile. regression tests passed. So I tested pg_restore on Windows and Linux. $ createdb test $ pgbench -i -s 1000 test $ pg_dump -Fc test > out $ createdb restore $ pg_restore -d restore out $ psql-c 'select max(aid) from accounts' restore max ----------- 100000000 (1 row) pg_restore was normally completed. Thank you for your great work. I wish that the patch will be committed. -- Yoshiyuki Asaba y-asaba@sraoss.co.jp
Yoshiyuki Asaba wrote: > From: Magnus Hagander <magnus@hagander.net> > Subject: Re: [HACKERS] pg_restore fails with a custom backup file > Date: Fri, 16 Feb 2007 10:13:35 +0100 > >> On Fri, Feb 16, 2007 at 02:09:41PM +0900, Yoshiyuki Asaba wrote: >> >>>>> Does not compile on my MinGW - errors in the system headers (unistd.h, >>>>> io.h) due to changing the argument format for chsize(). The change of >>>>> off_t propagated into parts of the system headers, thus chaos was >>>>> ensured. >>>>> >>>>> I still think we need to use a pgoff_t. Will look at combining these two >>>>> approaches. >>>> Here's a patch that tries this. >>>> *needs more testing*. But built with this patch, I can dump and >>>> restore a table at the end of a 10gb database without errors. >>> I tried the attached patch. But I got the following error. >>> >>> pg_backup_archiver.o(.text+0x1fa4): In function `allocAH': >>> C:/msys/1.0/home/y-asaba/postgresql-8.2.3-patch/src/bin/pg_dump/pg_backup_archiver.c:1580: undefined reference to `fseeko64' >>> ... >>> make[3]: *** [pg_dump] Error 1 >>> >>> $ uname -sr >>> MINGW32_NT-5.1 1.0.10(0.46/3/2) >>> >>> Is MINGW version too old? >> I think so. It seems this was added in version 1.24 of stdio.h in mingw >> (http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/mingw/include/stdio.h?cvsroot=src). >> Could you try upgrading mingw and see if that helps? Or possibly >> instlaling side-by-side a different version (if they even allow that)? > > OK. I have upgraded mingw and tried to compile. regression tests > passed. So I tested pg_restore on Windows and Linux. > > $ createdb test > $ pgbench -i -s 1000 test > $ pg_dump -Fc test > out > $ createdb restore > $ pg_restore -d restore out > $ psql -c 'select max(aid) from accounts' restore > max > ----------- > 100000000 > (1 row) > > pg_restore was normally completed. Thank you for your great work. I wish > that the patch will be committed. Thanks for running those tests. I need to test the msvc build as well, but I can hopefully do that quickly. (and a few tests that I didn't break unix) I'd also like a comment from at least one other "patch reviewer" that the methods used are good. //Magnus
Magnus Hagander <magnus@hagander.net> writes:
> I'd also like a comment from at least one other "patch reviewer" that
> the methods used are good.
It looks reasonable as far as it goes. One thought is that pg_dump
really should have noticed that it was writing a broken archive.
On machines where off_t is 32 bits, can't we detect the overflow
situation?
regards, tom lane
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> I'd also like a comment from at least one other "patch reviewer" that >> the methods used are good. > > It looks reasonable as far as it goes. One thought is that pg_dump Ok. I'll run some more tests and then get it in. > really should have noticed that it was writing a broken archive. > On machines where off_t is 32 bits, can't we detect the overflow > situation? IIRC, there was a warning from pg_dump. I don't recall exactly what, and don't have the space to re-run the test on my laptop here, but I think it was from: write_msg(modulename, "WARNING: ftell mismatch with expected position -- ftell used\n"); //Magnus
On Sat, Feb 17, 2007 at 08:40:54PM +0100, Magnus Hagander wrote: > > IIRC, there was a warning from pg_dump. I don't recall exactly what, and > don't have the space to re-run the test on my laptop here, but I think > it was from: > write_msg(modulename, "WARNING: ftell mismatch with expected position -- > ftell used\n"); Ok, I've confirmed that the output is this: D:\prog\pgsql\inst\bin>pg_dump -Fc -Z0 test > out pg_dump: [custom archiver] WARNING: ftell mismatch with expected position -- ftell used pg_dump: [custom archiver] WARNING: ftell mismatch with expected position -- ftell used pg_dump: [custom archiver] WARNING: ftell mismatch with expected position -- ftell used Three warnings for that one dump - my guess would be one for each table past the 2gb limit. //Magnus
On Sat, Feb 17, 2007 at 01:28:22PM -0500, Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: > > I'd also like a comment from at least one other "patch reviewer" that > > the methods used are good. > > It looks reasonable as far as it goes. One thought is that pg_dump > really should have noticed that it was writing a broken archive. > On machines where off_t is 32 bits, can't we detect the overflow > situation? Tested on MSVC as well, works. Also tested and doesn't break the build on Linux (which shouldn't be affected at all). So, patch applied to HEAD and 8.2. //Magnus