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(at)sraoss(dot)co(dot)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(at)momjian(dot)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

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)hagander(dot)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(at)hagander(dot)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(at)momjian(dot)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(at)guitar(dot)ocn(dot)ne(dot)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(at)sraoss(dot)co(dot)jp

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

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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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

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(at)hagander(dot)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(at)sraoss(dot)co(dot)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(at)hagander(dot)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(at)sraoss(dot)co(dot)jp

Yoshiyuki Asaba wrote:
> From: Magnus Hagander <magnus(at)hagander(dot)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(at)hagander(dot)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(at)hagander(dot)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(at)hagander(dot)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