Re: pg_dump and large files - is this a problem?

I have put the latest patch at:

http://downloads.rhyme.com.au/postgresql/pg_dump/

along with two dump files of the regression DB, one with 4 byte
and the other with 8 byte offsets. I can read/restore each from
the other, so it looks pretty good. Once the endianness is tested,
we should be OK.

Known problems:

- will not cope with > 4GB files and size_t not 64 bit.
- when printing data position, it is assumed that off_t is UINT64
(we could remove this entirely - it's just for display)
- if seek is not supported, then an intXX is assigned to off_t
when file offsets are needed. This *should* not cause a problem
since without seek, the offsets will not be written to the file.

Changes from Prior Version:

- No longer stores or outputs data length
- Assumes result of ftello is correct if it disagrees with internally
kept tally.
- 'pg_restore -l' now shows sizes of int and offset.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Philip Warner wrote:
>
> I have put the latest patch at:
>
> http://downloads.rhyme.com.au/postgresql/pg_dump/
>
> along with two dump files of the regression DB, one with 4 byte
> and the other with 8 byte offsets. I can read/restore each from
> the other, so it looks pretty good. Once the endianness is tested,
> we should be OK.
>
> Known problems:
>
> - will not cope with > 4GB files and size_t not 64 bit.
> - when printing data position, it is assumed that off_t is UINT64
> (we could remove this entirely - it's just for display)
> - if seek is not supported, then an intXX is assigned to off_t
> when file offsets are needed. This *should* not cause a problem
> since without seek, the offsets will not be written to the file.
>
> Changes from Prior Version:
>
> - No longer stores or outputs data length
> - Assumes result of ftello is correct if it disagrees with internally
> kept tally.
> - 'pg_restore -l' now shows sizes of int and offset.
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 09:18 PM 20/10/2002 -0400, Bruce Momjian wrote:
>I will try to apply it within the next 48 hours.

I'm happy to apply it when necessary; but I wouldn't do it until we've from
some someone with a big-endian machine...

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 09:18 PM 20/10/2002 -0400, Bruce Momjian wrote:
> >I will try to apply it within the next 48 hours.
>
> I'm happy to apply it when necessary; but I wouldn't do it until we've from
> some someone with a big-endian machine...

Well, I think Tom was going to try it on his HPUX machine. However, it
is on the open items list, so we are going to need to get it in there
soon anyway, or yank it all out. If no big endian people want to test
it, we will have to ship and then I am sure some big-ending testing will
happen. ;-)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 09:50 PM 20/10/2002 -0400, Bruce Momjian wrote:
>Well, I think Tom was going to try it on his HPUX machine.

It might be good if someone who knows a little more than me about
endianness etc has a look at the patch - specifically this bit of code:

#if __BYTE_ORDER == __LITTLE_ENDIAN
for (off = 0 ; off < sizeof(off_t) ; off++) {
#else
for (off = sizeof(off_t) -1 ; off >= 0 ; off--) {
#endif
i = *(char*)(ptr+off);
(*AH->WriteBytePtr) (AH, i);
}

It is *intended* to write the data such that the least significant byte
is written first to the file, but the dump Giles put on his FTP site
is not correct - it's written msb->lsb.

There seem to be two possibilities (a) I am an idiot and there is something
wrong with the code above that I can not see, or (b) the test:

#if __BYTE_ORDER == __LITTLE_ENDIAN

is not the right thing to do. Any insights would be appreciated.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner <pjw(at)rhyme(dot)com(dot)au> writes:
> It might be good if someone who knows a little more than me about
> endianness etc has a look at the patch - specifically this bit of code:

> #if __BYTE_ORDER == __LITTLE_ENDIAN

Well, the main problem with that is there's no such symbol as
__BYTE_ORDER ...

I'd prefer not to introduce one, either, if we can possibly avoid it.
I know that we have BYTE_ORDER defined in the port header files, but
I think it's quite untrustworthy, since there is no other place in the
main distribution that uses it anymore (AFAICS only contrib/pgcrypto
uses it at all).

The easiest way to write and reassemble an arithmetic value in a
platform-independent order is via shifting. For instance,

// write, LSB first
for (i = 0; i < sizeof(off_t); i++)
{
writebyte(val & 0xFF);
val >>= 8;
}

// read, LSB first
val = 0;
shift = 0;
for (i = 0; i < sizeof(off_t); i++)
{
val |= (readbyte() << shift);
shift += 8;
}

(This assumes readbyte delivers an unsigned byte, else you might need to
mask it with 0xFF before shifting.)

regards, tom lane

At 09:47 AM 21/10/2002 -0400, Tom Lane wrote:
>Well, the main problem with that is there's no such symbol as
>__BYTE_ORDER ...

What about just:

int i = 256;

then checking the first byte? This should give me the endianness, and makes
a non-destructive write (not sure it it's important). Currently the
commonly used code does not rely on off_t arithmetic, so if possible I'd
like to avoid shift. Does that sound reasonable? Or overly cautious?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner <pjw(at)rhyme(dot)com(dot)au> writes:
> then checking the first byte? This should give me the endianness, and makes
> a non-destructive write (not sure it it's important). Currently the
> commonly used code does not rely on off_t arithmetic, so if possible I'd
> like to avoid shift. Does that sound reasonable? Or overly cautious?

I think it's pointless. Let's assume off_t is not an arithmetic type
but some weird struct dreamed up by a crazed kernel hacker. What are
the odds that dumping the bytes in it, in either order, will produce
something that's compatible with any other platform? There could be
padding, or the fields might be in an order that doesn't match the
byte order within the fields, or something else.

The shift method requires *no* directly endian-dependent code,
and I think it will work on any platform where you have any hope of
portability anyway.

regards, tom lane

Here is a modified version of Philip's patch that has the changes Tom
suggested; treating off_t as an integral type. I did light testing on
my BSD/OS machine that has 8-byte off_t but I don't have 4 gigs of free
space to test larger files.

ftp://candle.pha.pa.us/pub/postgresql/mypatches/pg_dump

Can others test?

---------------------------------------------------------------------------

Tom Lane wrote:
> Philip Warner <pjw(at)rhyme(dot)com(dot)au> writes:
> > then checking the first byte? This should give me the endianness, and makes
> > a non-destructive write (not sure it it's important). Currently the
> > commonly used code does not rely on off_t arithmetic, so if possible I'd
> > like to avoid shift. Does that sound reasonable? Or overly cautious?
>
> I think it's pointless. Let's assume off_t is not an arithmetic type
> but some weird struct dreamed up by a crazed kernel hacker. What are
> the odds that dumping the bytes in it, in either order, will produce
> something that's compatible with any other platform? There could be
> padding, or the fields might be in an order that doesn't match the
> byte order within the fields, or something else.
>
> The shift method requires *no* directly endian-dependent code,
> and I think it will work on any platform where you have any hope of
> portability anyway.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

On Mon, 2002-10-21 at 20:47, Bruce Momjian wrote:
>
> Here is a modified version of Philip's patch that has the changes Tom
> suggested; treating off_t as an integral type. I did light testing on
> my BSD/OS machine that has 8-byte off_t but I don't have 4 gigs of free
> space to test larger files.
I can make an account for anyone that wants to play on UnixWare 7.1.3.

--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler(at)lerctr(dot)org
US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749

Larry Rosenman wrote:
> On Mon, 2002-10-21 at 20:47, Bruce Momjian wrote:
> >
> > Here is a modified version of Philip's patch that has the changes Tom
> > suggested; treating off_t as an integral type. I did light testing on
> > my BSD/OS machine that has 8-byte off_t but I don't have 4 gigs of free
> > space to test larger files.
> I can make an account for anyone that wants to play on UnixWare 7.1.3.

If you have 7.3, you can just test this way:

1) apply the patch
2) run the regression tests
3) pg_dump -Fc regression >/tmp/x
4) pg_restore -Fc </tmp/x

That's all I did and it worked.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

On Mon, 2002-10-21 at 20:52, Bruce Momjian wrote:
> Larry Rosenman wrote:
> > On Mon, 2002-10-21 at 20:47, Bruce Momjian wrote:
> > >
> > > Here is a modified version of Philip's patch that has the changes Tom
> > > suggested; treating off_t as an integral type. I did light testing on
> > > my BSD/OS machine that has 8-byte off_t but I don't have 4 gigs of free
> > > space to test larger files.
> > I can make an account for anyone that wants to play on UnixWare 7.1.3.
>
> If you have 7.3, you can just test this way:
I haven't had the time to play with 7.3 (busy on a NUMBER of other
things).

I'm more than willing to supply resources, just my time is short right
now.

>
> 1) apply the patch
> 2) run the regression tests
> 3) pg_dump -Fc regression >/tmp/x
> 4) pg_restore -Fc </tmp/x
>
> That's all I did and it worked.
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073
--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler(at)lerctr(dot)org
US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749

At 09:52 PM 21/10/2002 -0400, Bruce Momjian wrote:
> 4) pg_restore -Fc </tmp/x

pg_restore /tmp/x

is enough; it will determine the file type, and by avoiding the pipe, you
allow it to do seeks which are not much use here, but are usefull when you
only restore one table in a very large backup.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 10:16 AM 21/10/2002 -0400, Tom Lane wrote:
>What are
>the odds that dumping the bytes in it, in either order, will produce
>something that's compatible with any other platform?

None, but it will be compatible with itself (the most we can hope for), and
will work even if shifting is not supported for off_t (how likely is
that?). I agree shift is definitely the way to go if it works on arbitrary
data - ie. it does not rely on off_t being an integer. Can I shift a struct?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner <pjw(at)rhyme(dot)com(dot)au> writes:
> None, but it will be compatible with itself (the most we can hope for), and
> will work even if shifting is not supported for off_t (how likely is
> that?). I agree shift is definitely the way to go if it works on arbitrary
> data - ie. it does not rely on off_t being an integer. Can I shift a struct?

You can't. If there are any platforms where in fact off_t isn't an
arithmetic type, then shifting code would break there. I am not sure
there are any; can anyone provide a counterexample?

It would be simple enough to add a configure test to see whether off_t
is arithmetic (just try to compile "off_t x; x <<= 8;"). How about
#ifdef OFF_T_IS_ARITHMETIC_TYPE
// cross-platform compatible
use shifting method
#else
// not cross-platform compatible
read or write bytes of struct in storage order
#endif

regards, tom lane

Tom Lane wrote:
> Philip Warner <pjw(at)rhyme(dot)com(dot)au> writes:
> > None, but it will be compatible with itself (the most we can hope for), and
> > will work even if shifting is not supported for off_t (how likely is
> > that?). I agree shift is definitely the way to go if it works on arbitrary
> > data - ie. it does not rely on off_t being an integer. Can I shift a struct?
>
> You can't. If there are any platforms where in fact off_t isn't an
> arithmetic type, then shifting code would break there. I am not sure
> there are any; can anyone provide a counterexample?
>
> It would be simple enough to add a configure test to see whether off_t
> is arithmetic (just try to compile "off_t x; x <<= 8;"). How about
> #ifdef OFF_T_IS_ARITHMETIC_TYPE
> // cross-platform compatible
> use shifting method
> #else
> // not cross-platform compatible
> read or write bytes of struct in storage order
> #endif

It is my understanding that off_t is an integral type and fpos_t is
perhaps a struct. My fgetpos manual page says:

The fgetpos() and fsetpos() functions are alternate interfaces equivalent
to ftell() and fseek() (with whence set to SEEK_SET ), setting and stor-
ing the current value of the file offset into or from the object refer-
enced by pos. On some (non-UNIX) systems an ``fpos_t'' object may be a
complex object and these routines may be the only way to portably reposi-
tion a text stream.

I poked around and found this Usenet posting:

http://groups.google.com/groups?q=C+off_t+standard+integral&hl=en&lr=&ie=UTF-8&oe=UTF-8&selm=E958tG.8tH%40root.co.uk&rnum=1

stating that while off_t must be arithmetic, it doesn't have to be
integral, meaning it could be float or double, which can't be shifted.

However, since we don't know if we support any non-integral off_t
platforms, and because a configure test would require us to have two
code paths for with/without integral off_t, I suggest we apply my
version of Philip's patch and let's see if everyone can compile it
cleanly. It does have the advantage of being more portable on systems
that do have integral off_t, which I think is most/all of our supported
platforms.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 12:00 PM 22/10/2002 -0400, Bruce Momjian wrote:
>It does have the advantage of being more portable on systems
>that do have integral off_t

I suspect it is no more portable than determining storage order by using
'int i = 256', then writing in storage order, and has the disadvantage that
it may break as discussed.

AFAICT, using storage order will not break under any circumstances within
one OS/architecture (unlike using shift), and will not break any more often
than using shift in cases where off_t is integral.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> However, since we don't know if we support any non-integral off_t
> platforms, and because a configure test would require us to have two
> code paths for with/without integral off_t, I suggest we apply my
> version of Philip's patch and let's see if everyone can compile it
> cleanly.

Actually, it looks to me like configure will spit up if off_t is not
an integral type:

/* Check that off_t can represent 2**63 - 1 correctly.
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];

So I think we're wasting our time to debate whether we need to support
non-integral off_t ... let's just apply Bruce's version and wait to
see if anyone has a problem before doing more work.

regards, tom lane

Philip Warner wrote:
> At 12:00 PM 22/10/2002 -0400, Bruce Momjian wrote:
> >It does have the advantage of being more portable on systems
> >that do have integral off_t
>
> I suspect it is no more portable than determining storage order by using
> 'int i = 256', then writing in storage order, and has the disadvantage that
> it may break as discussed.
>
> AFAICT, using storage order will not break under any circumstances within
> one OS/architecture (unlike using shift), and will not break any more often
> than using shift in cases where off_t is integral.

Your version will break more often because we are assuming we can
determine the endian-ness of the OS, _and_ for quad off_t types,
assuming we know that is stored the same too. While we have ending for
int's, I have no idea if quads are always stored the same. By accessing
it as an integral type, we make certain it is output the same way every
time for every OS.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > However, since we don't know if we support any non-integral off_t
> > platforms, and because a configure test would require us to have two
> > code paths for with/without integral off_t, I suggest we apply my
> > version of Philip's patch and let's see if everyone can compile it
> > cleanly.
>
> Actually, it looks to me like configure will spit up if off_t is not
> an integral type:
>
> /* Check that off_t can represent 2**63 - 1 correctly.
> We can't simply define LARGE_OFF_T to be 9223372036854775807,
> since some C++ compilers masquerading as C compilers
> incorrectly reject 9223372036854775807. */
> #define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> && LARGE_OFF_T % 2147483647 == 1)
> ? 1 : -1];
>
> So I think we're wasting our time to debate whether we need to support
> non-integral off_t ... let's just apply Bruce's version and wait to
> see if anyone has a problem before doing more work.

I am concerned about one more thing. On BSD/OS, we have off_t of quad
(8 byte), but we don't have fseeko, so this call looks questionable:

if (fseeko(AH->FH, tctx->dataPos, SEEK_SET) != 0)

In this case, dataPos is off_t (8 bytes), while fseek only accepts long
in that parameter (4 bytes). When this code is hit, a file > 4 gigs
will seek to the wrong offset, I am afraid. Also, I don't understand
why the compiler doesn't produce a warning.

I wonder if I should add a conditional test so this code is hit only if
HAVE_FSEEKO is defined. There is alternative code for all the non-zero
fseeks.

Comments?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Your version will break more often because we are assuming we can
> determine the endian-ness of the OS, _and_ for quad off_t types,
> assuming we know that is stored the same too. While we have ending for
> int's, I have no idea if quads are always stored the same.

There is precedent for problems of that ilk, too, cf PDP_ENDIAN: years
ago someone made double-word-integer software routines and did not
think twice about which word should appear first in storage, with the
consequence that the storage order was neither little-endian nor
big-endian. (We have exactly the same issue with our CRC routines for
compilers without int64: the two-int32 struct is defined in a way that's
compatible with little-endian storage, and on a big-endian machine it'll
produce a funny storage order.)

Unless someone can point to a supported (or potentially interesting)
platform on which off_t is indeed not integral, I think the shift-based
code is our safest bet. (The precedent of the off_t checking code in
configure makes me really doubt that there are any platforms with
non-integral off_t.)

regards, tom lane

Patch applied with shift <</>> changes by me. Thanks.

---------------------------------------------------------------------------

Philip Warner wrote:
>
> I have put the latest patch at:
>
> http://downloads.rhyme.com.au/postgresql/pg_dump/
>
> along with two dump files of the regression DB, one with 4 byte
> and the other with 8 byte offsets. I can read/restore each from
> the other, so it looks pretty good. Once the endianness is tested,
> we should be OK.
>
> Known problems:
>
> - will not cope with > 4GB files and size_t not 64 bit.
> - when printing data position, it is assumed that off_t is UINT64
> (we could remove this entirely - it's just for display)
> - if seek is not supported, then an intXX is assigned to off_t
> when file offsets are needed. This *should* not cause a problem
> since without seek, the offsets will not be written to the file.
>
> Changes from Prior Version:
>
> - No longer stores or outputs data length
> - Assumes result of ftello is correct if it disagrees with internally
> kept tally.
> - 'pg_restore -l' now shows sizes of int and offset.
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:
> > So I think we're wasting our time to debate whether we need to support
> > non-integral off_t ... let's just apply Bruce's version and wait to
> > see if anyone has a problem before doing more work.
>
> I am concerned about one more thing. On BSD/OS, we have off_t of quad
> (8 byte), but we don't have fseeko, so this call looks questionable:
>
> if (fseeko(AH->FH, tctx->dataPos, SEEK_SET) != 0)
>
> In this case, dataPos is off_t (8 bytes), while fseek only accepts long
> in that parameter (4 bytes). When this code is hit, a file > 4 gigs
> will seek to the wrong offset, I am afraid. Also, I don't understand
> why the compiler doesn't produce a warning.
>
> I wonder if I should add a conditional test so this code is hit only if
> HAVE_FSEEKO is defined. There is alternative code for all the non-zero
> fseeks.

Here is a patch that I think fixes the problem I outlined above. If
there is no fseeko(), it will not call fseek with a non-zero offset
unless sizeof(off_t) <= sizeof(long).

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian writes:

> I am concerned about one more thing. On BSD/OS, we have off_t of quad
> (8 byte), but we don't have fseeko, so this call looks questionable:
>
> if (fseeko(AH->FH, tctx->dataPos, SEEK_SET) != 0)

Maybe you want to ask your OS provider how the heck this is supposed to
work. I mean, it's great to have wide types, but what's the point if the
API can't handle them?

--
Peter Eisentraut peter_e(at)gmx(dot)net

Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > I am concerned about one more thing. On BSD/OS, we have off_t of quad
> > (8 byte), but we don't have fseeko, so this call looks questionable:
> >
> > if (fseeko(AH->FH, tctx->dataPos, SEEK_SET) != 0)
>
> Maybe you want to ask your OS provider how the heck this is supposed to
> work. I mean, it's great to have wide types, but what's the point if the
> API can't handle them?

Excellent question. They do have fsetpos/fgetpos, and I think they
think you are supposed to use those. However, they don't do seek from
current position, and they don't take an off_t, so I am confused myself.

I did ask on the mailing list and everyone kind of agreed it was a
missing feature. However, because of the way we call fseeko not knowing
if it is a quad or a long, I think we have to add the checks to prevent
such wild seeks from happening.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 05:37 PM 22/10/2002 -0400, Bruce Momjian wrote:
>! if (ctx->hasSeek
>! #if !defined(HAVE_FSEEKO)
>! && sizeof(off_t) <= sizeof(long)
>! #endif
>! )

Just to clarify my understanding:

- HAVE_FSEEKO is tested & defined in configure
- If it is not defined, then all calls to fseeko will magically be
translated to fseek calls, and use the 'long' parameter type.

Is that right?

If so, why don't we:

#if defined(HAVE_FSEEKO)
#define FILE_OFFSET off_t
#define FSEEK fseeko
#else
#define FILE_OFFSET long
#define FSEEK fseek
#end if

then replace all refs to off_t with FILE_OFFSET, and fseeko with FSEEK.

Existing checks etc will then refuse to load file offsets with significant
bytes after the 4th byte, we will still use fseek/o in broken OS
implementations of off_t.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 05:37 PM 22/10/2002 -0400, Bruce Momjian wrote:
> >! if (ctx->hasSeek
> >! #if !defined(HAVE_FSEEKO)
> >! && sizeof(off_t) <= sizeof(long)
> >! #endif
> >! )
>
> Just to clarify my understanding:
>
> - HAVE_FSEEKO is tested & defined in configure
> - If it is not defined, then all calls to fseeko will magically be
> translated to fseek calls, and use the 'long' parameter type.
>
> Is that right?
>
> If so, why don't we:
>
> #if defined(HAVE_FSEEKO)
> #define FILE_OFFSET off_t
> #define FSEEK fseeko
> #else
> #define FILE_OFFSET long
> #define FSEEK fseek
> #end if
>
> then replace all refs to off_t with FILE_OFFSET, and fseeko with FSEEK.
>
> Existing checks etc will then refuse to load file offsets with significant
> bytes after the 4th byte, we will still use fseek/o in broken OS
> implementations of off_t.

Uh, not exactly. I have off_t as a quad, and I don't have fseeko, so
the above conditional doesn't work. I want to use off_t, but can't use
fseek(). As it turns out, the code already has options to handle no
fseek, so it seems to work anyway. I think what you miss may be the
table of contents in the archive, if I am reading the code correctly.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 10:46 PM 22/10/2002 -0400, Bruce Momjian wrote:
>Uh, not exactly. I have off_t as a quad, and I don't have fseeko, so
>the above conditional doesn't work. I want to use off_t, but can't use
>fseek().

Then when you create dumps, they will be invalid since I assume that ftello
is also broken in the same way. You need to fix _getFilePos as well. And
any other place that uses an off_t needs to be looked at very carefully.
The code was written assuming that if 'hasSeek' was set, then we could
trust it.

Given that you say you do have support for some kind of 64 bt offset, I
would be a lot happier with these changes if you did something akin to my
original sauggestion:

#if defined(HAVE_FSEEKO)
#define FILE_OFFSET off_t
#define FSEEK fseeko
#elseif defined(HAVE_SOME_OTHER_FSEEK)
#define FILE_OFFSET some_other_offset
#define FSEEK some_other_fseek
#else
#define FILE_OFFSET long
#define FSEEK fseek
#end if

...assuming you have a non-broken 64 bit fseek/tell pair, then this will
work in all cases, and make the code a lot less ugly (assuming of course
the non-broken version can be shifted).

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Sounds messy. Let me see if I can code up an fseeko/ftello for BSD/OS
and add that to /port. No reason to hold up beta for that, though.

I wonder if any other platforms have this limitation. I think we need
to add some type of test for no-fseeko()/ftello() and sizeof(off_t) >
sizeof(long). This fseeko/ftello/off_t is just too fluid, and the
failure modes too serious.

---------------------------------------------------------------------------

Philip Warner wrote:
> At 10:46 PM 22/10/2002 -0400, Bruce Momjian wrote:
> >Uh, not exactly. I have off_t as a quad, and I don't have fseeko, so
> >the above conditional doesn't work. I want to use off_t, but can't use
> >fseek().
>
> Then when you create dumps, they will be invalid since I assume that ftello
> is also broken in the same way. You need to fix _getFilePos as well. And
> any other place that uses an off_t needs to be looked at very carefully.
> The code was written assuming that if 'hasSeek' was set, then we could
> trust it.
>
> Given that you say you do have support for some kind of 64 bt offset, I
> would be a lot happier with these changes if you did something akin to my
> original sauggestion:
>
> #if defined(HAVE_FSEEKO)
> #define FILE_OFFSET off_t
> #define FSEEK fseeko
> #elseif defined(HAVE_SOME_OTHER_FSEEK)
> #define FILE_OFFSET some_other_offset
> #define FSEEK some_other_fseek
> #else
> #define FILE_OFFSET long
> #define FSEEK fseek
> #end if
>
> ...assuming you have a non-broken 64 bit fseek/tell pair, then this will
> work in all cases, and make the code a lot less ugly (assuming of course
> the non-broken version can be shifted).
>
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I wonder if any other platforms have this limitation. I think we need
> to add some type of test for no-fseeko()/ftello() and sizeof(off_t) >
> sizeof(long). This fseeko/ftello/off_t is just too fluid, and the
> failure modes too serious.

I am wondering why pg_dump has to depend on either fseek or ftell.

regards, tom lane

At 12:32 AM 23/10/2002 -0400, Tom Lane wrote:
>I am wondering why pg_dump has to depend on either fseek or ftell.

It doesn't - it just works better and has more features if they are
available, much like zlib etc.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 12:29 AM 23/10/2002 -0400, Bruce Momjian wrote:
>This fseeko/ftello/off_t is just too fluid, and the
>failure modes too serious.

I agree. Can you think of a better solution than the one I suggested???

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

OK, you are saying if we don't have fseeko(), there is no reason to use
off_t, and we may as well use long. What limitations does that impose,
and are the limitations clear to the user.

What has me confused is that I only see two places that use a non-zero
fseeko, and in those cases, there is a non-fseeko code path that does
the same thing, or the call isn't actually required. Both cases are in
pg_dump/pg_dump_custom.c. It appears seeking in the file is an
optimization that prevents all the blocks from being read. That is
fine, but we shouldn't introduce failure cases to do that.

If BSD/OS is the only problem OS, I can deal with that, but I have no
idea if other OS's have the same limitation, and because of the way our
code exists now, we are not even checking to see if there is a problem.

I did some poking around, and on BSD/OS, fgetpos/fsetpos use fpos_t,
which is actually off_t, and interestingly, lseek() uses off_t too.
Seems only fseek/ftell is limited to long. I can easily implemnt
fseeko/ftello using fgetpos/fsetpos, but that is only one OS.

One idea would be to patch up BSD/OS in backend/port/bsdi and add a
configure tests that actually fails if fseeko doesn't exist _and_
sizeof(off_t) > sizeof(long). That would at least catch OS's before
they make >2gig backups that can't be restored.

---------------------------------------------------------------------------

Philip Warner wrote:
> At 10:46 PM 22/10/2002 -0400, Bruce Momjian wrote:
> >Uh, not exactly. I have off_t as a quad, and I don't have fseeko, so
> >the above conditional doesn't work. I want to use off_t, but can't use
> >fseek().
>
> Then when you create dumps, they will be invalid since I assume that ftello
> is also broken in the same way. You need to fix _getFilePos as well. And
> any other place that uses an off_t needs to be looked at very carefully.
> The code was written assuming that if 'hasSeek' was set, then we could
> trust it.
>
> Given that you say you do have support for some kind of 64 bt offset, I
> would be a lot happier with these changes if you did something akin to my
> original sauggestion:
>
> #if defined(HAVE_FSEEKO)
> #define FILE_OFFSET off_t
> #define FSEEK fseeko
> #elseif defined(HAVE_SOME_OTHER_FSEEK)
> #define FILE_OFFSET some_other_offset
> #define FSEEK some_other_fseek
> #else
> #define FILE_OFFSET long
> #define FSEEK fseek
> #end if
>
> ...assuming you have a non-broken 64 bit fseek/tell pair, then this will
> work in all cases, and make the code a lot less ugly (assuming of course
> the non-broken version can be shifted).
>
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 01:02 AM 23/10/2002 -0400, Bruce Momjian wrote:

>OK, you are saying if we don't have fseeko(), there is no reason to use
>off_t, and we may as well use long. What limitations does that impose,
>and are the limitations clear to the user.

What I'm saying is that if we have not got fseeko then we should use any
'seek-class' function that returns a 64 bit value. We have already made the
assumption that off_t is an integer; the same logic that came to that
conclusion, applies just as validly to the other seek functions.

Secondly, if there is no 64 bit 'seek-class' function, then we should
probably use a size_t, but a long would probably be fine too. I am not
particularly attached to this part; long, int etc etc. Whatever is most
likely to return an integer and work with whatever function we choose.

As to implications: assuming they are all integers (which as you know I
don't like), we should have no problems.

If a system does not have any function to access 64 bit file offsets, then
I'd say they are pretty unlikely to have files > 2GB.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 01:02 AM 23/10/2002 -0400, Bruce Momjian wrote:
>
> >OK, you are saying if we don't have fseeko(), there is no reason to use
> >off_t, and we may as well use long. What limitations does that impose,
> >and are the limitations clear to the user.
>
> What I'm saying is that if we have not got fseeko then we should use any
> 'seek-class' function that returns a 64 bit value. We have already made the
> assumption that off_t is an integer; the same logic that came to that
> conclusion, applies just as validly to the other seek functions.

Oh, I see, so try to use fsetpos/fgetpos? I can write wrappers for
those to look like fgetpos/fsetpos and put it in /port.

> Secondly, if there is no 64 bit 'seek-class' function, then we should
> probably use a size_t, but a long would probably be fine too. I am not
> particularly attached to this part; long, int etc etc. Whatever is most
> likely to return an integer and work with whatever function we choose.
>
> As to implications: assuming they are all integers (which as you know I
> don't like), we should have no problems.
>
> If a system does not have any function to access 64 bit file offsets, then
> I'd say they are pretty unlikely to have files > 2GB.

OK, my OS can handle 64-bit files, but has only fgetpos/fsetpos, so I
could get that working. The bigger question is what about OS's that
have 64-bit off_t/files but don't have any seek-type functions. I did
research to find mine, but what about others that may have other
variants?

I think you are right that we have to not use off_t and use long if we
can't find a proper 64-bit seek function, but what are the failure modes
of doing this? Exactly what happens for larger files?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Philip Warner wrote:
> At 01:02 AM 23/10/2002 -0400, Bruce Momjian wrote:
>
> >OK, you are saying if we don't have fseeko(), there is no reason to use
> >off_t, and we may as well use long. What limitations does that impose,
> >and are the limitations clear to the user.
>
> What I'm saying is that if we have not got fseeko then we should use any
> 'seek-class' function that returns a 64 bit value. We have already made the
> assumption that off_t is an integer; the same logic that came to that
> conclusion, applies just as validly to the other seek functions.
>
> Secondly, if there is no 64 bit 'seek-class' function, then we should
> probably use a size_t, but a long would probably be fine too. I am not
> particularly attached to this part; long, int etc etc. Whatever is most
> likely to return an integer and work with whatever function we choose.
>
> As to implications: assuming they are all integers (which as you know I
> don't like), we should have no problems.
>
> If a system does not have any function to access 64 bit file offsets, then
> I'd say they are pretty unlikely to have files > 2GB.

Let me see if I can be clearer. With shifting off_t, if that fails, we
will find out right away, at compile time. I think that is acceptable.

What I am concerned about are cases that fail at runtime, specifically
during a restore of a >2gig file. In my reading of the code, those
failures will be silent or will produce unusual error messages. I don't
think we can ship code that has strange failure modes for data restore.

Now, if someone knows those failure cases, I would love to hear about
it. If not, I will dig into the code today and find out where they are.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian writes:

> I think you are right that we have to not use off_t and use long if we
> can't find a proper 64-bit seek function, but what are the failure modes
> of doing this? Exactly what happens for larger files?

First we need to decide what we want to happen and after that think about
how to implement it. Given sizeof(off_t) > sizeof(long) and no fseeko(),
we have the following options:

1. Disable access to large files.

2. Seek in some other way.

What's it gonna be?

--
Peter Eisentraut peter_e(at)gmx(dot)net

Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > I think you are right that we have to not use off_t and use long if we
> > can't find a proper 64-bit seek function, but what are the failure modes
> > of doing this? Exactly what happens for larger files?
>
> First we need to decide what we want to happen and after that think about
> how to implement it. Given sizeof(off_t) > sizeof(long) and no fseeko(),
> we have the following options:
>
> 1. Disable access to large files.
>
> 2. Seek in some other way.
>
> What's it gonna be?

OK, well BSD/OS now works, but I wonder if there are any other quad
off_t OS's out there without fseeko.

How would we disable access to large files? Do we fstat the file and
see if it is too large? I suppose we are looking for cases where the
file system has large files, but fseeko doesn't allow us to access them.
Should we leave this issue alone and wait to find another OS with this
problem, and we can then rejigger fseeko.c to handle that OS too?

Looking at the pg_dump code, it seems the fseeks are optional in there
anyway because it already has code to read the file sequentially rather
than use fseek, and the TOC case in pg_backup_custom.c says that is
optional too.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> First we need to decide what we want to happen and after that think about
> how to implement it. Given sizeof(off_t) > sizeof(long) and no fseeko(),
> we have the following options:

It seems obvious to me that there are no platforms that offer
sizeof(off_t) > sizeof(long) but have no API for doing seeks with off_t.
That would be just plain silly. IMHO it's acceptable for us to fail at
configure time if we can't figure out how to seek.

The question is *which* seek APIs we need to support. Are there any
besides fseeko() and fgetpos()?

regards, tom lane

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> How would we disable access to large files?

I think configure should fail if it can't find a way to seek.
Workaround for anyone in that situation is configure --disable-largefile.

regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > First we need to decide what we want to happen and after that think about
> > how to implement it. Given sizeof(off_t) > sizeof(long) and no fseeko(),
> > we have the following options:
>
> It seems obvious to me that there are no platforms that offer
> sizeof(off_t) > sizeof(long) but have no API for doing seeks with off_t.
> That would be just plain silly. IMHO it's acceptable for us to fail at
> configure time if we can't figure out how to seek.

I would certainly be happy failing at configure time, so we know at the
start what is broken, rather than failures during restore.

> The question is *which* seek APIs we need to support. Are there any
> besides fseeko() and fgetpos()?

What I have added is BSD/OS specific because only on BSD/OS do I know
fpos_t and off_t are the same type. If we come up with other platforms,
we will have to deal with it then.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 05:50 PM 23/10/2002 -0400, Bruce Momjian wrote:
>Looking at the pg_dump code, it seems the fseeks are optional in there
>anyway because it already has code to read the file sequentially rather

But there are features that are not available if it can't seek: eg. it will
not restore in a different order to that in which it was written; it will
not dump data offsets in the TOC so dump files can not be restored in
alternate orders; restore times will be large for a single table (it has to
read the entire file potentially).

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 05:50 PM 23/10/2002 -0400, Bruce Momjian wrote:
> >Looking at the pg_dump code, it seems the fseeks are optional in there
> >anyway because it already has code to read the file sequentially rather
>
> But there are features that are not available if it can't seek: eg. it will
> not restore in a different order to that in which it was written; it will
> not dump data offsets in the TOC so dump files can not be restored in
> alternate orders; restore times will be large for a single table (it has to
> read the entire file potentially).

OK, that helps. We just got a list of 2 other OS's without fseeko and
with large file support. Any NetBSD before Auguest 2002 has that
problem. We are going to need to either get fseeko workarounds for
those, or disable those features in a meaningful way.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 10:42 AM 23/10/2002 -0400, Bruce Momjian wrote:
>What I am concerned about are cases that fail at runtime, specifically
>during a restore of a >2gig file.

Please give an example that would still apply assuming we get a working
seek/tell pair that works with whatever we use as an offset?

If you are concerned about reading a dump file with 8 byte offsets on a
machine with 4 byte off_t, that case and it's permutations are already covered.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 10:42 AM 23/10/2002 -0400, Bruce Momjian wrote:
> >What I am concerned about are cases that fail at runtime, specifically
> >during a restore of a >2gig file.
>
> Please give an example that would still apply assuming we get a working
> seek/tell pair that works with whatever we use as an offset?

If we get this, everything is fine. I have done that for BSD/OS today.
I may need to do the same for NetBSD/OpenBSD too.

> If you are concerned about reading a dump file with 8 byte offsets on a
> machine with 4 byte off_t, that case and it's permutations are already covered.

No, I know that is covered because it will report a proper error message
on the restore on the 4-byte off_t machine.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 11:50 PM 23/10/2002 +0200, Peter Eisentraut wrote:

>1. Disable access to large files.
>
>2. Seek in some other way.

This gets my vote, but I would like to see a clean implementation (not huge
quantities if ifdefs every time we call fseek); either we write our own
fseek as Bruce seems to be suggesting, or we have a single header file that
defines the FSEEK/FTELL/OFF_T to point to the 'right' functions, where
'right' is defined as 'most likely to generate an integer and which makes
use of the largest number of bytes'.

The way the code is currently written it does not matter if this is a 16 or
3 byte value - so long as it is an integer.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 11:50 PM 23/10/2002 +0200, Peter Eisentraut wrote:
>
> >1. Disable access to large files.
> >
> >2. Seek in some other way.
>
> This gets my vote, but I would like to see a clean implementation (not huge
> quantities if ifdefs every time we call fseek); either we write our own
> fseek as Bruce seems to be suggesting, or we have a single header file that
> defines the FSEEK/FTELL/OFF_T to point to the 'right' functions, where
> 'right' is defined as 'most likely to generate an integer and which makes
> use of the largest number of bytes'.

We have to write another function because fsetpos doesn't do SEEK_CUR so
you have to implement it with more complex code. It isn't a drop in
place thing.

> The way the code is currently written it does not matter if this is a 16 or
> 3 byte value - so long as it is an integer.

Right. What we are assuming now is that off_t can be seeked using
whatever we defined for fseeko, which is incorrect in one, and now I
hear more than one OS.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 09:36 PM 23/10/2002 -0400, Bruce Momjian wrote:
>We are going to need to either get fseeko workarounds for
>those, or disable those features in a meaningful way.

????? if we have not got a 64 bit seek function of any kind, then use a 32
bit seek - the features don't need to be disabled. AFAICT, this is a
non-issue: no 64 bit seek means no large files.

I'm not sure we should even worry about it, but if you are genuinely
concerned that we have no 64 bit seek call, but we do have files > 4GB,
then If you really want to disable seek, just modify the code that sets
'hasSeek' - don't screw around with every seek call. But only modify clear
it if the file is > 4GB.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 09:41 PM 23/10/2002 -0400, Bruce Momjian wrote:
>If we get this, everything is fine. I have done that for BSD/OS today.
>I may need to do the same for NetBSD/OpenBSD too.

What did you do to achieve this?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 09:41 PM 23/10/2002 -0400, Bruce Momjian wrote:
> >If we get this, everything is fine. I have done that for BSD/OS today.
> >I may need to do the same for NetBSD/OpenBSD too.
>
> What did you do to achieve this?

See src/port/fseeko.c in current CVS, with some configure.in glue.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 09:45 PM 23/10/2002 -0400, Bruce Momjian wrote:
>We have to write another function because fsetpos doesn't do SEEK_CUR so
>you have to implement it with more complex code. It isn't a drop in
>place thing.

The only code that uses SEEK_CUR is the code to check if seek is available
- I am ver happy to change that to SEEK_SET - I can't even recall why I
used SEEK_CUR. The code that does the real seeks uses SEEK_SET.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 11:55 AM 24/10/2002 +1000, Philip Warner wrote:

>The only code that uses SEEK_CUR is the code to check if seek is available
>- I am ver happy to change that to SEEK_SET - I can't even recall why I
>used SEEK_CUR. The code that does the real seeks uses SEEK_SET.

Come to think of it:

ctx->hasSeek = (fseeko(AH->FH, 0, SEEK_CUR) == 0);

should be replaced by:

#ifdef HAS_FSEEK[O]
ctx->hasSeek = TRUE;
#else
ctx->hasSeek = FALSE;
#endif

Since we're now checking for it in configure, we should remove the checks
from the pg_dump code.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 09:45 PM 23/10/2002 -0400, Bruce Momjian wrote:
> >We have to write another function because fsetpos doesn't do SEEK_CUR so
> >you have to implement it with more complex code. It isn't a drop in
> >place thing.
>
> The only code that uses SEEK_CUR is the code to check if seek is available
> - I am ver happy to change that to SEEK_SET - I can't even recall why I
> used SEEK_CUR. The code that does the real seeks uses SEEK_SET.

There are other problems. fgetpos() expects a pointer to an fpos_t,
while ftello just returns off_t, so you need a local variable in the
function to pass to fgetpos() and they return that from the function.

It is much cleaner to just duplicate the entire API so you don't have
any limitations or failure cases.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Well, that certainly changes the functionality of the code. I thought
that fseeko test was done so that things that couldn't be seeked on were
detected. Not sure what isn't seek-able, maybe named pipes. I thought
it was testing that so I didn't touch that variable.

This was my original thought, that we have non-fseeko code in place.
Can we just trigger the non-fseeko code on HAS_FSEEKO. The code would
be something like:

if (sizeof(long) >= sizeof(off_t))
ctx->hasSeek = TRUE;
else
#ifdef HAVE_FSEEKO
ctx->hasSeek = TRUE;
#else
ctx->hasSeek = FALSE;
#endif

---------------------------------------------------------------------------

Philip Warner wrote:
> At 11:55 AM 24/10/2002 +1000, Philip Warner wrote:
>
> >The only code that uses SEEK_CUR is the code to check if seek is available
> >- I am ver happy to change that to SEEK_SET - I can't even recall why I
> >used SEEK_CUR. The code that does the real seeks uses SEEK_SET.
>
> Come to think of it:
>
> ctx->hasSeek = (fseeko(AH->FH, 0, SEEK_CUR) == 0);
>
> should be replaced by:
>
> #ifdef HAS_FSEEK[O]
> ctx->hasSeek = TRUE;
> #else
> ctx->hasSeek = FALSE;
> #endif
>
> Since we're now checking for it in configure, we should remove the checks
> from the pg_dump code.
>
>
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 10:08 PM 23/10/2002 -0400, Bruce Momjian wrote:
>Well, that certainly changes the functionality of the code. I thought
>that fseeko test was done so that things that couldn't be seeked on were
>detected.

You are quite correct. It should read:

#ifdef HAVE_FSEEKO
ctx->hasSeek = fseeko(...,SEEK_SET);
#else
ctx->hasSeek = FALSE;
#endif

pipes are the main case for which we are checking.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

At 10:03 PM 23/10/2002 -0400, Bruce Momjian wrote:
>It is much cleaner to just duplicate the entire API so you don't have
>any limitations or failure cases.

We may still end up using macros in pg_dump to cope with cases where off_t
& fseeko are not defined - if there are any. I presume we would then just
revert to calling fseek/ftell etc.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

Philip Warner wrote:
> At 10:03 PM 23/10/2002 -0400, Bruce Momjian wrote:
> >It is much cleaner to just duplicate the entire API so you don't have
> >any limitations or failure cases.
>
> We may still end up using macros in pg_dump to cope with cases where off_t
> & fseeko are not defined - if there are any. I presume we would then just
> revert to calling fseek/ftell etc.

Well, we have fseeko falling back to fseek already, so that is working
fine. I don't think we will find any OS's without off_t. We just need
a little smarts. Let me see if I can work on it now.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Philip Warner wrote:
> At 10:08 PM 23/10/2002 -0400, Bruce Momjian wrote:
> >Well, that certainly changes the functionality of the code. I thought
> >that fseeko test was done so that things that couldn't be seeked on were
> >detected.
>
> You are quite correct. It should read:
>
> #ifdef HAVE_FSEEKO
> ctx->hasSeek = fseeko(...,SEEK_SET);
> #else
> ctx->hasSeek = FALSE;
> #endif
>
> pipes are the main case for which we are checking.

OK, I have applied the following patch to set hasSeek only if
fseek/fseeko is reliable. This takes care of the random failure case
for large files. Now I need to see if I can get the custom fseeko
working for more platforms.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

The patch will not work. Please reread my quoted email.

At 09:32 PM 24/10/2002 -0400, Bruce Momjian wrote:
>Philip Warner wrote:
> >
> > You are quite correct. It should read:
> >
> > #ifdef HAVE_FSEEKO
> > ctx->hasSeek = fseeko(...,SEEK_SET);
> > #else
> > ctx->hasSeek = FALSE;
> > #endif
> >
> > pipes are the main case for which we are checking.
>
>OK, I have applied the following patch to set hasSeek only if
>fseek/fseeko is reliable.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

You are going to have to be more specific than that.

---------------------------------------------------------------------------

Philip Warner wrote:
>
> The patch will not work. Please reread my quoted email.
>
> At 09:32 PM 24/10/2002 -0400, Bruce Momjian wrote:
> >Philip Warner wrote:
> > >
> > > You are quite correct. It should read:
> > >
> > > #ifdef HAVE_FSEEKO
> > > ctx->hasSeek = fseeko(...,SEEK_SET);
> > > #else
> > > ctx->hasSeek = FALSE;
> > > #endif
> > >
> > > pipes are the main case for which we are checking.
> >
> >OK, I have applied the following patch to set hasSeek only if
> >fseek/fseeko is reliable.
>
>
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

At 09:56 PM 24/10/2002 -0400, Bruce Momjian wrote:
> > > > You are quite correct. It should read:
> > > >
> > > > #ifdef HAVE_FSEEKO
> > > > ctx->hasSeek = fseeko(...,SEEK_SET);

^^^^^^^^^^^^^^^^^^^^^^

> > > > #else
> > > > ctx->hasSeek = FALSE;
> > > > #endif

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

OK, finally figured it out. I had used fseek instead of fseeko.

---------------------------------------------------------------------------

Philip Warner wrote:
> At 09:56 PM 24/10/2002 -0400, Bruce Momjian wrote:
> > > > > You are quite correct. It should read:
> > > > >
> > > > > #ifdef HAVE_FSEEKO
> > > > > ctx->hasSeek = fseeko(...,SEEK_SET);
>
> ^^^^^^^^^^^^^^^^^^^^^^
>
>
> > > > > #else
> > > > > ctx->hasSeek = FALSE;
> > > > > #endif
>
> ----------------------------------------------------------------
> Philip Warner | __---_____
> Albatross Consulting Pty. Ltd. |----/ - \
> (A.B.N. 75 008 659 498) | /(@) ______---_
> Tel: (+61) 0500 83 82 81 | _________ \
> Fax: (+61) 0500 83 82 82 | ___________ |
> Http://www.rhyme.com.au | / \|
> | --________--
> PGP key available upon request, | /
> and from pgp5.ai.mit.edu:11371 |/
>
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073