Re: [pgsql-hackers-win32] snprintf causes regression tests to fail

Nicolai Tufar wrote:
> Regression test diff is attached.
> It fails on the following tests:
> int8
> subselect
> union
> sequence
>
> It fails to display correctly number "4567890123456789".
> In output is shows "-869367531". Apparent overflow or
> interpreting int8 as int4.
>
> while rewriting snprintf() I did not touch the actual functions
> that convert number to ASCII digit string. In truth, if you
> force PostgreSQL to use snprintf.c without my patch applied
> it produces the same errors.
>
> What can be wrong? GCC bug? The one I use is:
> gcc.exe (GCC) 3.3.1 (mingw special 20030804-1)

I can confirm your failure in current sources on Win32:

template1=# create table test(x int8);
CREATE TABLE
template1=# insert into test values ('4567890123456789');
INSERT 17235 1
template1=# select * from test;
x
------------
-869367531
(1 row)

and it knows it is a large number:

template1=# select * from test where x > 1000::int8;
x
------------
-869367531
(1 row)
template1=# select * from test where x < 1000::int8;
x
---
(0 rows)

I am going to add some debugs to see what is being passed to *printf().

--
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:
> I can confirm your failure in current sources on Win32:
>
> template1=# create table test(x int8);
> CREATE TABLE
> template1=# insert into test values ('4567890123456789');
> INSERT 17235 1
> template1=# select * from test;
> x
> ------------
> -869367531
> (1 row)
>
> and it knows it is a large number:
>
> template1=# select * from test where x > 1000::int8;
> x
> ------------
> -869367531
> (1 row)
> template1=# select * from test where x < 1000::int8;
> x
> ---
> (0 rows)
>
> I am going to add some debugs to see what is being passed to *printf().

I have not found the solution yet but am heading to bed. My next guess
is that Win32 isn't handling va_arg(..., long long int) properly.

--
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 Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian
> My next guess
> is that Win32 isn't handling va_arg(..., long long int) properly.
>

I am trying various combination of number and types
of parameters in my test program and everything prints fine.
When it comes to pg, it fails :(

> > template1=# select * from test where x > 1000::int8;
> > x
> > ------------
> > -869367531
> > (1 row)

I am not too fluent in source code, could someone
point me to there actual call to snprintf() is being done
when a query like this is executed. I could not find it myslef
:(

Regards,
Nick

Nicolai Tufar wrote:
> On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian
> > My next guess
> > is that Win32 isn't handling va_arg(..., long long int) properly.
> >
>
> I am trying various combination of number and types
> of parameters in my test program and everything prints fine.
> When it comes to pg, it fails :(
>
> > > template1=# select * from test where x > 1000::int8;
> > > x
> > > ------------
> > > -869367531
> > > (1 row)
>
> I am not too fluent in source code, could someone
> point me to there actual call to snprintf() is being done
> when a query like this is executed. I could not find it myslef

Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():

if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)

and that calls port/snprintf.c.

I have added a puts() in snprintf.c to make sure it is getting the
long/long specifier.

--
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

I spent all day debugging it. Still have absolutely
no idea what could possibly go wrong. Does
anyone have a slightest clue what can it be and
why it manifests itself only on win32?

On Tue, 1 Mar 2005 09:29:07 -0500 (EST), Bruce Momjian
<pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
> Nicolai Tufar wrote:
> > On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian
> > > My next guess
> > > is that Win32 isn't handling va_arg(..., long long int) properly.
> > >
> >
> > I am trying various combination of number and types
> > of parameters in my test program and everything prints fine.
> > When it comes to pg, it fails :(
> >
> > > > template1=# select * from test where x > 1000::int8;
> > > > x
> > > > ------------
> > > > -869367531
> > > > (1 row)
> >
> > I am not too fluent in source code, could someone
> > point me to there actual call to snprintf() is being done
> > when a query like this is executed. I could not find it myslef
>
> Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():
>
> if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)
>
> and that calls port/snprintf.c.
>
> I have added a puts() in snprintf.c to make sure it is getting the
> long/long specifier.
>
> --
> 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
>

> Nicolai Tufar wrote:
>> On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian
>> > My next guess
>> > is that Win32 isn't handling va_arg(..., long long int) properly.
>> >
>>
>> I am trying various combination of number and types
>> of parameters in my test program and everything prints fine.
>> When it comes to pg, it fails :(
>>
>> > > template1=# select * from test where x > 1000::int8;
>> > > x
>> > > ------------
>> > > -869367531
>> > > (1 row)
>>
>> I am not too fluent in source code, could someone
>> point me to there actual call to snprintf() is being done
>> when a query like this is executed. I could not find it myslef
>
> Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():
>
> if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)
>
> and that calls port/snprintf.c.
>
> I have added a puts() in snprintf.c to make sure it is getting the
> long/long specifier.

Just a question, and don't mind me if I am being rude, isn't this the
WRONG PLACE for a "printf" function? Wouldn't an "itoa" function be more
efficient and be less problematic?

pgsql(at)mohawksoft(dot)com writes:
> Just a question, and don't mind me if I am being rude, isn't this the
> WRONG PLACE for a "printf" function? Wouldn't an "itoa" function be more
> efficient and be less problematic?

This particular call could be so replaced, but it wouldn't solve the
general problem. snprintf has to work.

regards, tom lane

> I spent all day debugging it. Still have absolutely
> no idea what could possibly go wrong. Does
> anyone have a slightest clue what can it be and
> why it manifests itself only on win32?

It may be that the CLIB has badly broken support for 64bit integers on 32
bit platforms. Does anyone know of any Cygwin/Ming issues?

Is this only with the new snprintf code in Win32?

Is this a problem with snprintf as implemented in src/port?

Is there a reason why we don't use the snprintf that comes with the
various C compilers?

>
>
> On Tue, 1 Mar 2005 09:29:07 -0500 (EST), Bruce Momjian
> <pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
>> Nicolai Tufar wrote:
>> > On Tue, 1 Mar 2005 00:55:20 -0500 (EST), Bruce Momjian
>> > > My next guess
>> > > is that Win32 isn't handling va_arg(..., long long int) properly.
>> > >
>> >
>> > I am trying various combination of number and types
>> > of parameters in my test program and everything prints fine.
>> > When it comes to pg, it fails :(
>> >
>> > > > template1=# select * from test where x > 1000::int8;
>> > > > x
>> > > > ------------
>> > > > -869367531
>> > > > (1 row)
>> >
>> > I am not too fluent in source code, could someone
>> > point me to there actual call to snprintf() is being done
>> > when a query like this is executed. I could not find it myslef
>>
>> Sure, in src/backend/utils/adt/int8.c, there is a call in int8out():
>>
>> if ((len = snprintf(buf, MAXINT8LEN, INT64_FORMAT, val)) < 0)
>>
>> and that calls port/snprintf.c.
>>
>> I have added a puts() in snprintf.c to make sure it is getting the
>> long/long specifier.
>>
>> --
>> 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
>>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>

On Tue, 1 Mar 2005 15:38:58 -0500 (EST), pgsql(at)mohawksoft(dot)com
<pgsql(at)mohawksoft(dot)com> wrote:
> Is there a reason why we don't use the snprintf that comes with the
> various C compilers?

snprintf() is usually buried in OS libraries. We implement
our own snprintf to make things like this:
snprintf(buf,"%2$s %1$s","world","Hello");
which is not supported on some platforms work.

We do it for national language translation of
messages. In some languages you may need
to change order of parameters to make a meaningful
sentence.

Another question is why we are using it for printing
values from database. I am not too good on function
overriding in standard C but maybe there is a way
to usage of standard snprintf() in a particular place.

> On Tue, 1 Mar 2005 15:38:58 -0500 (EST), pgsql(at)mohawksoft(dot)com
> <pgsql(at)mohawksoft(dot)com> wrote:
>> Is there a reason why we don't use the snprintf that comes with the
>> various C compilers?
>
> snprintf() is usually buried in OS libraries. We implement
> our own snprintf to make things like this:
> snprintf(buf,"%2$s %1$s","world","Hello");
> which is not supported on some platforms work.
>
> We do it for national language translation of
> messages. In some languages you may need
> to change order of parameters to make a meaningful
> sentence.
>
> Another question is why we are using it for printing
> values from database. I am not too good on function
> overriding in standard C but maybe there is a way
> to usage of standard snprintf() in a particular place.
>

Well, here is a stupid question, do we even know which snprintf we are
using on Win32? May it be possible that we are using the MingW version
which may be broken?

pgsql(at)mohawksoft(dot)com writes:
> Well, here is a stupid question, do we even know which snprintf we are
> using on Win32? May it be possible that we are using the MingW version
> which may be broken?

The regression tests were not failing until we started using the port/
version ...

regards, tom lane

On Tue, 1 Mar 2005 16:20:50 -0500 (EST), pgsql(at)mohawksoft(dot)com
<pgsql(at)mohawksoft(dot)com> wrote:
>
> Well, here is a stupid question, do we even know which snprintf we are
> using on Win32? May it be possible that we are using the MingW version
> which may be broken?

Defenitely not. I checked it by placing debugging print
statements in code.