Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 16 Mar 2005 01:00:21 -0500 (EST), Bruce Momjian pgman@candle.pha.pa.us wrote: I have applied a modified version of your patch, attached. I am so sorry, I sent untested patch again. Thank you very much for patience in fixing it. The patch looks perfectly fine and works under Solaris. Under win32 I am still struggling with build environment. In many directories link fails with undefined reference to `pg_snprintf' in other it fails with undefined reference to `_imp__libintl_sprintf'. In yet another directory it fails with both, like in src/interfaces/ecpg/pgtypeslib: dlltool --export-all --output-def pgtypes.def numeric.o datetime.o common.o dt_common.o timestamp.o interval.o pgstrcasecmp.o dllwrap -o libpgtypes.dll --dllname libpgtypes.dll --def pgtypes.def numeric.o datetime.o common.o dt_common.o timestamp.o interval.o pgstrcasecmp.o -L../../../../src/port -lm numeric.o(.text+0x19ea):numeric.c: undefined reference to `_imp__libintl_sprintf' datetime.o(.text+0x476):datetime.c: undefined reference to `pg_snprintf' common.o(.text+0x1cd):common.c: undefined reference to `pg_snprintf' common.o(.text+0x251):common.c: undefined reference to `pg_snprintf' dt_common.o(.text+0x538):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x553):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x597):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x5d5):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x628):dt_common.c: undefined reference to `_imp__libintl_sprintf' dt_common.o(.text+0x7e8):dt_common.c: more undefined references to `_imp__libintl_sprintf' follow c:\MinGW\bin\dllwrap.exe: c:\MinGW\bin\gcc exited with status 1 make: *** [libpgtypes.a] Error 1 Could someone with a better grasp of configure and win32 environment check it? Aparently no one regularily compiles source code under win32 during development cycle these days. Best regards, Nicolai ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian pgman@candle.pha.pa.us writes: I am wondering if we should just process long long args and %$ args, and pass everything else to the native snprintf. AFAICS this is a non-starter --- how will you construct the call to snprintf? Or even vsnprintf? C doesn't provide the tools you need to make it happen. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian pgman@candle.pha.pa.us writes: Is there no way to create a va_arg va_list structure in C? Exactly. The standard lets you *read out* from such a structure, but there's no provision for creating one on-the-fly. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Resubmission of yesterday's patch so that it would
cont conflict with Bruce's cvs commit. Pleas apply.
Best regards,
Nicolai.
On Sat, 12 Mar 2005 01:58:15 +0200, Nicolai Tufar [EMAIL PROTECTED] wrote:
On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian
pgman@candle.pha.pa.us wrote:
The CVS-tip implementation is fundamentally broken and won't work even
for our internal uses. I've not wasted time complaining about it
because I thought we were going to replace it. If we can't find a
usable replacement then we're going to have to put a lot of effort
into fixing what's there. On the whole I think the effort would be
better spent importing someone else's solution.
Oh, so our existing implementation doesn't even meet our needs. OK.
Which made me wander why did I not aggree with
Tom Lane's suggestion to make do three passes
instead of two. Tom was right, as usual. It happened to
be much easier than I expected. The patch is attached.
Please apply.
Tom, what do you think? Will it be fine with you?
Best regards,
Nicolai
*** ./src/port/snprintf.c.orig Sat Mar 12 09:13:43 2005
--- ./src/port/snprintf.c Sat Mar 12 10:01:44 2005
***
*** 195,200
--- 195,202
int pointflag;
char func;
int realpos;
+ int longflag;
+ int longlongflag;
} *fmtpar, **fmtparptr;
/* Create enough structures to hold all arguments */
***
*** 264,275
--- 266,279
realpos = position;
len = 0;
goto nextch;
+ /*
case '*':
if (pointflag)
maxwidth = va_arg(args, int);
else
len = va_arg(args, int);
goto nextch;
+ */
case '.':
pointflag = 1;
goto nextch;
***
*** 301,316
#endif
case 'u':
case 'U':
! /* fmtnum(value,base,dosign,ljust,len,zpad,output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 305,312
#endif
case 'u':
case 'U':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 325,340
break;
case 'o':
case 'O':
! /* fmtnum(value,base,dosign,ljust,len,zpad,output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 321,328
break;
case 'o':
case 'O':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 349,365
break;
case 'd':
case 'D':
! if (longflag)
! {
! if (longlongflag)
! {
! value = va_arg(args, int64);
! }
! else
! value = va_arg(args, long);
! }
! else
! value = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 337,344
break;
case 'd':
case 'D':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 373,387
fmtpos++;
break;
case 'x':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 352,359
fmtpos++;
break;
case 'x':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 395,409
fmtpos++;
break;
case 'X':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
!
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian
pgman@candle.pha.pa.us wrote:
The CVS-tip implementation is fundamentally broken and won't work even
for our internal uses. I've not wasted time complaining about it
because I thought we were going to replace it. If we can't find a
usable replacement then we're going to have to put a lot of effort
into fixing what's there. On the whole I think the effort would be
better spent importing someone else's solution.
Oh, so our existing implementation doesn't even meet our needs. OK.
Which made me wander why did I not aggree with
Tom Lane's suggestion to make do three passes
instead of two. Tom was right, as usual. It happened to
be much easier than I expected. The patch is attached.
Please apply.
Tom, what do you think? Will it be fine with you?
Best regards,
Nicolai
*** ./src/port/snprintf.c.orig Sat Mar 12 01:28:49 2005
--- ./src/port/snprintf.c Sat Mar 12 01:08:30 2005
***
*** 195,200
--- 195,202
int pointflag;
char func;
int realpos;
+ int longflag;
+ int longlongflag;
} *fmtpar, **fmtparptr;
/* Create enough structures to hold all arguments */
***
*** 263,274
--- 265,278
realpos = position;
len = 0;
goto nextch;
+ /*
case '*':
if (pointflag)
maxwidth = va_arg(args, int);
else
len = va_arg(args, int);
goto nextch;
+ */
case '.':
pointflag = 1;
goto nextch;
***
*** 300,315
#endif
case 'u':
case 'U':
! /* fmtnum(value,base,dosign,ljust,len,zpad,output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 304,311
#endif
case 'u':
case 'U':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 324,339
break;
case 'o':
case 'O':
! /* fmtnum(value,base,dosign,ljust,len,zpad,output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 320,327
break;
case 'o':
case 'O':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 348,364
break;
case 'd':
case 'D':
! if (longflag)
! {
! if (longlongflag)
! {
! value = va_arg(args, int64);
! }
! else
! value = va_arg(args, long);
! }
! else
! value = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 336,343
break;
case 'd':
case 'D':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 372,386
fmtpos++;
break;
case 'x':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 351,358
fmtpos++;
break;
case 'x':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
***
*** 394,408
fmtpos++;
break;
case 'X':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].numvalue = value;
--- 366,373
fmtpos++;
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 9 Mar 2005 22:51:27 -0500 (EST), Bruce Momjian pgman@candle.pha.pa.us wrote: What do you think about it? Shall I abandon FreeBSD and go ahead Incorporating Trio? Yes, maybe just add the proper %$ handling from Trio to what we have now. Adding proper %$ from Trio will require too much effort. I would rather not do it. Not because I am lazy but because: 1) Trio team seem to be very serious about standards, update the library as soon as new standards come out: quote Trio fully implements the C99 (ISO/IEC 9899:1999) and UNIX98 (the Single Unix Specification, Version 2) standards, as well as many features from other implementations, e.g. the GNU libc and BSD4. /quote 2) If we integrate the whole library in source code we will not have to maintain it and will rely on Trio team for bug fixes and updates. Integrating it will be very easy since all of the functions begin with trio_. I used it instead of the src/port/snrpintf.c one and it passes regression tests under Win32 just fine. The downside is that Trio library is rather big. It is 3 .c and 6 .h files totalling 11556 lines. Compiled it is 71224 bytes not stripped and 56204 bytes stripped on Solaris 10 for x86, 32-bit. Even for a shared library it will probably be too much. Trio has a lot of string handling functions which are probably not necessary. Would you like me to try to remove everything unnecessary from it or we will settle with the full version? Regards, Nicolai Tufar ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian pgman@candle.pha.pa.us writes: Please see my posting about using a macro for snprintf. If the current implementation of snprintf is enough for our existing translation users we probably don't need to add anything more to it because snprintf will not be exported to client applications. The CVS-tip implementation is fundamentally broken and won't work even for our internal uses. I've not wasted time complaining about it because I thought we were going to replace it. If we can't find a usable replacement then we're going to have to put a lot of effort into fixing what's there. On the whole I think the effort would be better spent importing someone else's solution. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Tom Lane wrote: The CVS-tip implementation is fundamentally broken and won't work even for our internal uses. I've not wasted time complaining about it because I thought we were going to replace it. If we can't find a usable replacement then we're going to have to put a lot of effort into fixing what's there. On the whole I think the effort would be better spent importing someone else's solution. Bruce Momjian wrote: Oh, so our existing implementation doesn't even meet our needs. OK. Very well, I too, tend to think that importing some else's solution is the way to go. Tom, have you looked at Trio? If it is fine with you too, I will strip it to the bare minimum needed for snprintf(), vsnprintf() and printf() by Saturday. Regards, Nicolai Tufar ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Nicolai Tufar [EMAIL PROTECTED] writes: Very well, I too, tend to think that importing some else's solution is the way to go. Tom, have you looked at Trio? I have not seen it ... but if it looks reasonable to you, have a go at it. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Fri, 11 Mar 2005 01:14:31 -0500, Tom Lane wrote: Nicolai Tufar [EMAIL PROTECTED] writes: Very well, I too, tend to think that importing some else's solution is the way to go. Tom, have you looked at Trio? I have not seen it ... but if it looks reasonable to you, have a go at it. It looks reasonable to me. It claims to: fully implement C99 (ISO/IEC 9899:1999) and UNIX98 (the Single Unix Specification, Version 2) standards, as well as many features from other implementations, e.g. the GNU libc and BSD4. I compiled and run regression tests with it used instead of our current implementation and it worked fine under win32 and Solaris x86. The only problem is that it is 11000 lines as oposed to our curent implementation's 600. I will remove everything unnecessary and submit a patch for consideration. Regards, Nicolai Tufar ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Dear all, After struggling for one week to to integrate FreeBSD's vfprintf.c into PostgreSQL I finally gave up. It is too dependent on underlying FreeBSD system functions. To incorporate it into PostgreSQL we need to move vfprintf.c file itself, two dozen files form gdtoa and a half a dozen __XXtoa.c files scattered in apparently random fashion all around FreeBSD source tree. Instead I researched some other implementations of snprintf on the web released under a license compatible with PostgreSQL's. The most suitable one I have come upon is Trio [http://daniel.haxx.se/projects/trio/]. It is distributed under a MIT-like license which, I think will be compatible with us. What do you think about it? Shall I abandon FreeBSD and go ahead ncorporatng Tro? And by the way, what s the concluson of snprntf() vs. pg_snprintf() and UNIX libraries discussion a week ago? Which one shall I implement? Regards, Nicolai Tufar ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Wed, 2 Mar 2005 09:24:32 +0100, Joerg Hessdoerfer [EMAIL PROTECTED] wrote: don't know if PG borrowed the code from here, but according to the manpage FreeBSD 5.3 seems to have a quite complete implementation, see http://www.freebsd.org/cgi/man.cgi?query=snprintfapropos=0sektion=3manpath=FreeBSD+5.3-RELEASE+and+Portsformat=html Would this help? Yes, it would. With Tom Lane's blessing I am starting on incorporating it into PG now. ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: First line of thought: we surely must not insert a snprintf into libpq.so unless it is 100% up to spec *and* has no performance issues ... neither of which can be claimed of the CVS-tip version. Agreed, and we have to support all the 64-bit specifications a port might support like %qd and %I64d as well as %lld. I have added that to our current CVS version. I really dislike that idea and request that you revert it. Is there any way we can have just gettext() call our snprintf under a special name? The issue only comes up in libpq --- in the backend there is no reason that snprintf can't be our snprintf, and likewise in self-contained programs like psql. It might be worth the pain-in-the-neck quality to have libpq refer to the functions as pq_snprintf etc. Perhaps we could do this via macros #define snprintfpq_snprintf and not have to uglify the source code. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
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@candle.pha.pa.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@candle.pha.pa.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
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
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? Yes. Is this a problem with snprintf as implemented in src/port? Yes. Only. It works with the snprintf() in the runtime (this particular part). Is there a reason why we don't use the snprintf that comes with the various C compilers? It does not support positional parameters (I think it's called) which is required for proper translations. We do use that one when it works... //Magnus ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
On Tue, 1 Mar 2005 15:38:58 -0500 (EST), [EMAIL PROTECTED] [EMAIL PROTECTED] 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. ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail
[EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
