Thread: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT
The following bug has been logged on the website: Bug reference: 15080 Logged by: jonathan allen Email address: jallen@americansavingslife.com PostgreSQL version: 10.2 Operating system: Windows 10 x64 Description: I'm trying to access a bigint column on windows using ecpg and I get the following error: SQL error: unsupported type "long long" on line x I found ecpg_get_data() is the call generating the error, and there's a block of code being skipped because HAVE_LONG_LONG_INT is not defined. I found the following code in pg_config.h and tried adding the 3rd line (#define HAVE_LONG_LONG_INT 1)... /* Define to 1 if `long long int' works and is 64 bits. */ #if (_MSC_VER > 1200) #define HAVE_LONG_LONG_INT_64 1 #define HAVE_LONG_LONG_INT 1 #endif Then I tried recompiling postgres from source and copying over the new bin, lib, include and shared directories...but no luck, my program is still throwing the same error. I asked on the #postgresql irc channel and RhodiumToad and xocolatl said that if I reported this as a bug paquier would see this and he'd be the person to ask about it. I'd really like to be able to get a fix going that I can compile and use myself until a new public release is available - this bug has brought our postgres migration to a standstill, it's a real show stopper. :( Thank you for looking into this and hopefully pointing me in the right direction. :) -Jonathan
> I found ecpg_get_data() is the call generating the error, and there's > a > block of code being skipped because HAVE_LONG_LONG_INT is not > defined. > > I found the following code in pg_config.h and tried adding the 3rd > line > (#define HAVE_LONG_LONG_INT 1)... > > /* Define to 1 if `long long int' works and is 64 bits. */ > #if (_MSC_VER > 1200) > #define HAVE_LONG_LONG_INT_64 1 > #define HAVE_LONG_LONG_INT 1 > #endif ECPG does not use pg_config.h, try making your change in ecpg_config.h instead. > throwing the same error. I asked on the #postgresql irc channel and > RhodiumToad and xocolatl said that if I reported this as a bug > paquier would > see this and he'd be the person to ask about it. Well, I'm not Michael Paquier and I don't know know why he would be the person to ask about this, but I guess you don't mind me answering in his stead. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
>>>>> "Michael" == Michael Meskes <meskes@postgresql.org> writes: Michael> ECPG does not use pg_config.h, try making your change in Michael> ecpg_config.h instead. Aha. Solution.pm has this: if (IsNewer( 'src/interfaces/ecpg/include/ecpg_config.h', 'src/interfaces/ecpg/include/ecpg_config.h.in')) { print "Generating ecpg_config.h...\n"; open(my $o, '>', 'src/interfaces/ecpg/include/ecpg_config.h') || confess "Could not open ecpg_config.h"; print $o <<EOF; #if (_MSC_VER > 1200) #define HAVE_LONG_LONG_INT_64 1 #define ENABLE_THREAD_SAFETY 1 EOF print $o "#endif\n"; close($o); } from which HAVE_LONG_LONG_INT seems to be suspiciously missing? -- Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > Solution.pm has this: > #if (_MSC_VER > 1200) > #define HAVE_LONG_LONG_INT_64 1 > #define ENABLE_THREAD_SAFETY 1 > #endif > from which HAVE_LONG_LONG_INT seems to be suspiciously missing? I remember noticing that discrepancy recently when I was chasing something else about int64 support. It certainly looks wrong, but not being much of an ecpg user I was hesitant to touch it. regards, tom lane
> Solution.pm has this: > ... > from which HAVE_LONG_LONG_INT seems to be suspiciously missing? Right, but it is missing in pg_config.h, too, right? I have no personal experience with the Windows build, so I cannot tell if it should be defined. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Michael Meskes <meskes@postgresql.org> writes: >> Solution.pm has this: >> ... >> from which HAVE_LONG_LONG_INT seems to be suspiciously missing? > Right, but it is missing in pg_config.h, too, right? No, it does get defined on Unix builds (if appropriate), both in pg_config.h and ecpg_config.h. As far as I can see in a quick grep, the core code doesn't use that symbol anywhere anyway ... but ecpg does. It's the fact that ecpg_config.h gets the symbol defined on Unix builds but not MSVC builds that Andrew is on about. regards, tom lane
I wrote: > Michael Meskes <meskes@postgresql.org> writes: >> Right, but it is missing in pg_config.h, too, right? > No, it does get defined on Unix builds (if appropriate), both in > pg_config.h and ecpg_config.h. Oh, wait, now I see what you meant: there is no stanza like #if (_MSC_VER > 1200) #define HAVE_LONG_LONG_INT 1 #endif in pg_config.h.win32. I agree that there should be. As I said, none of the core backend cares at the moment ... but someday, that discrepancy is going to bite us. regards, tom lane
Right now I'm using a custom build of ecpg to enable Bigint support, but it would suuuuure be nice to have that working inthe next release of postgres. I agree, all that's missing is the #define HAVE_LONG_LONG_INT 1 line. :) -Jonathan -----Original Message----- From: Tom Lane [mailto:tgl@sss.pgh.pa.us] Sent: Saturday, February 24, 2018 1:12 PM To: Michael Meskes <meskes@postgresql.org> Cc: Andrew Gierth <andrew@tao11.riddles.org.uk>; Jonathan Allen <jallen@americansavingslife.com>; pgsql-bugs@lists.postgresql.org Subject: Re: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT I wrote: > Michael Meskes <meskes@postgresql.org> writes: >> Right, but it is missing in pg_config.h, too, right? > No, it does get defined on Unix builds (if appropriate), both in > pg_config.h and ecpg_config.h. Oh, wait, now I see what you meant: there is no stanza like #if (_MSC_VER > 1200) #define HAVE_LONG_LONG_INT 1 #endif in pg_config.h.win32. I agree that there should be. As I said, none of the core backend cares at the moment ... but someday,that discrepancy is going to bite us. regards, tom lane
Jonathan Allen <jallen@americansavingslife.com> writes: > Right now I'm using a custom build of ecpg to enable Bigint support, but it would suuuuure be nice to have that workingin the next release of postgres. I agree, all that's missing is the #define HAVE_LONG_LONG_INT 1 line. :) I'm afraid to push this in today, because today is a release wrap day, and there's no time to recover if it turns out that we tickle some portability issue. But I think we should fix it as soon as the release dust settles. regards, tom lane
I wrote: > I'm afraid to push this in today, because today is a release wrap day, > and there's no time to recover if it turns out that we tickle some > portability issue. But I think we should fix it as soon as the release > dust settles. Pushed now. Barring buildfarm complaints, it'll be in our May releases. regards, tom lane
Thank you! Thank you so very much. Using my own private build of ecpg until May will be a bit of a pain but hey - whatam I complaining about, right? I very much look forward to an official build of ecpg that properly handles Bigint rightout of the box. Thanks again for getting that worked out and released into the build pipeline, -Jonathan -----Original Message----- From: Tom Lane [mailto:tgl@sss.pgh.pa.us] Sent: Tuesday, February 27, 2018 2:48 PM To: Jonathan Allen <jallen@americansavingslife.com> Cc: Michael Meskes <meskes@postgresql.org>; Andrew Gierth <andrew@tao11.riddles.org.uk>; pgsql-bugs@lists.postgresql.org Subject: Re: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT I wrote: > I'm afraid to push this in today, because today is a release wrap day, > and there's no time to recover if it turns out that we tickle some > portability issue. But I think we should fix it as soon as the > release dust settles. Pushed now. Barring buildfarm complaints, it'll be in our May releases. regards, tom lane
I saw the release of v10.4 today and was very excited to try using the official version of ecpg, but unfortunately I'm getting the same "unsupported type "long long" on line x" error. SQL State: YE000, SQL Code: -200.
...did this fix not make it into the May release?
I guess I'm back to my custom build again. ☹
-Jonathan
-----Original Message-----
From: Jonathan Allen <jallen@americansavingslife.com>
Sent: Tuesday, February 27, 2018 3:59 PM
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Meskes <meskes@postgresql.org>; Andrew Gierth <andrew@tao11.riddles.org.uk>; pgsql-bugs@lists.postgresql.org
Subject: RE: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT
[This sender failed our fraud detection checks and may not be who they appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
Thank you! Thank you so very much. Using my own private build of ecpg until May will be a bit of a pain but hey - what am I complaining about, right? I very much look forward to an official build of ecpg that properly handles Bigint right out of the box.
Thanks again for getting that worked out and released into the build pipeline,
-Jonathan
-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Tuesday, February 27, 2018 2:48 PM
To: Jonathan Allen <jallen@americansavingslife.com>
Cc: Michael Meskes <meskes@postgresql.org>; Andrew Gierth <andrew@tao11.riddles.org.uk>; pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT
I wrote:
> I'm afraid to push this in today, because today is a release wrap day,
> and there's no time to recover if it turns out that we tickle some
> portability issue. But I think we should fix it as soon as the
> release dust settles.
Pushed now. Barring buildfarm complaints, it'll be in our May releases.
regards, tom lane
Jonathan Allen <jallen@americansavingslife.com> writes: > I saw the release of v10.4 today and was very excited to try using the official version of ecpg, but unfortunately I'mgetting the same "unsupported type "long long" on line x" error. SQL State: YE000, SQL Code: -200. > ...did this fix not make it into the May release? Well, we committed *something* about that: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fda3e65786763bd43abc576a23035a4cd24ed138 Does that not match the fix you were using? regards, tom lane
From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: Friday, May 11, 2018 12:36 PM > To: Jonathan Allen <jallen@americansavingslife.com> > Cc: Michael Meskes <meskes@postgresql.org>; Andrew Gierth > <andrew@tao11.riddles.org.uk>; pgsql-bugs@lists.postgresql.org > Subject: Re: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT > > Jonathan Allen <jallen@americansavingslife.com> writes: > > I saw the release of v10.4 today and was very excited to try using the > official version of ecpg, but unfortunately I'm getting the same > "unsupported type "long long" on line x" error. SQL State: YE000, SQL Code: > -200. > > ...did this fix not make it into the May release? > > Well, we committed *something* about that: > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fda > 3e65786763bd43abc576a23035a4cd24ed138 > > Does that not match the fix you were using? I confirmed that the above commit fix the case of not define HAVE_LONG_LONG_INT. But We found another case that causes "unsupported type "long long"" error in Windows environment. In our case, We got the same error "unsupported type ...", because of the flag HAVE_STRTOLL and HAVE_STRTOULL are not defined (*1). I created a patch which defines the above two flags in Visual Studio 2013 or greater. # The two functions strtoll and strtoull are support from Visual Studio 2013 Please confirm the attached. (*1) In our case, we got the error because the below code was ignored. src/interfaces/ecpg/ecpglib/data.c -------------- ... #ifdef HAVE_LONG_LONG_INT #ifdef HAVE_STRTOLL case ECPGt_long_long: *((long long int *) (var + offset * act_tuple)) = strtoll(pval, &scan_length, 10); if (garbage_left(isarray, &scan_length, compat)) { ecpg_raise(lineno, ECPG_INT_FORMAT, ECPG_SQLSTATE_DATATYPE_MISMATCH, pval); return (false); } pval = scan_length; break; #endif /* HAVE_STRTOLL */ #ifdef HAVE_STRTOULL case ECPGt_unsigned_long_long: *((unsigned long long int *) (var + offset * act_tuple)) = strtoull(pval, &scan_length, 10); if (garbage_left(isarray, &scan_length, compat)) { ecpg_raise(lineno, ECPG_UINT_FORMAT, ECPG_SQLSTATE_DATATYPE_MISMATCH, pval); return (false); } pval = scan_length; break; #endif /* HAVE_STRTOULL */ #endif /* HAVE_LONG_LONG_INT */ ... -------------- Thanks and best regards, --- Dang Minh Huong NEC Solution Innovators, Ltd. http://www.nec-solutioninnovators.co.jp/en/
Attachment
Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes: > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] >> Well, we committed *something* about that: >> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fda3e65786763bd43abc576a23035a4cd24ed138 >> Does that not match the fix you were using? > I confirmed that the above commit fix the case of not define HAVE_LONG_LONG_INT. > But We found another case that causes "unsupported type "long long"" error in > Windows environment. > In our case, We got the same error "unsupported type ...", because of the flag > HAVE_STRTOLL and HAVE_STRTOULL are not defined (*1). Ah! I looked through the uses of ECPG_UNSUPPORTED, and that seems to be the only other thing that needs to be covered. > I created a patch which defines the above two flags in Visual Studio 2013 or greater. > # The two functions strtoll and strtoull are support from Visual Studio 2013 > Please confirm the attached. It seems fairly unfortunate that this patch does not fix the problem for as far back as MSVC has "long long" support. From what I can tell, we could use _strtoi64 and _strtoui64 on older Windows versions. So I'm imagining something like #if (_MSC_VER > 1200) #define HAVE_LONG_LONG_INT_64 1 #endif ... #ifdef HAVE_LONG_LONG_INT_64 #define HAVE_STRTOLL 1 /* Before VS2013, use Microsoft's nonstandard equivalent function */ #if (_MSC_VER < 1800) #define strtoll _strtoi64 #endif #endif and similarly for strtoull. Please check that and see if it works. BTW, is it possible to set up an ecpg test case to verify that this stuff works? It'd have to handle platforms without long long though, so I'm not sure how to deal with that. regards, tom lane
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > It seems fairly unfortunate that this patch does not fix the problem for > as far back as MSVC has "long long" support. From what I can tell, we could > use _strtoi64 and _strtoui64 on older Windows versions. Thanks. You are right. We also should fix for older VS too. > something like > > #if (_MSC_VER > 1200) > #define HAVE_LONG_LONG_INT_64 1 > #endif > > ... > > #ifdef HAVE_LONG_LONG_INT_64 > #define HAVE_STRTOLL 1 > /* Before VS2013, use Microsoft's nonstandard equivalent function */ #if > (_MSC_VER < 1800) #define strtoll _strtoi64 #endif #endif > > and similarly for strtoull. > > Please check that and see if it works. Thanks, as you mentioned the attached works fine for me. I think the code is also fine for the before VS2013 but I not yet tested. > BTW, is it possible to set up an ecpg test case to verify that this stuff > works? > It'd have to handle platforms without long long though, so I'm not > sure how to deal with that. Yes. I was expecting that at least bigint will works fine with sqlda in linux system but it seem did not? Attached test patch works fine in Windows, but in Linux system the sqlda->sqlvar[i].sqltype return ECPGt_long but not ECPGt_long_long (as expected) when reference to bigint column. Is this another problem in linux? Or am I wrong something? Thanks and best regards, --- Dang Minh Huong NEC Solution Innovators, Ltd. http://www.nec-solutioninnovators.co.jp/en/
Attachment
> From: Huong Dangminh [mailto:huo-dangminh@ys.jp.nec.com] > > something like > > > > #if (_MSC_VER > 1200) > > #define HAVE_LONG_LONG_INT_64 1 > > #endif > > > > ... > > > > #ifdef HAVE_LONG_LONG_INT_64 > > #define HAVE_STRTOLL 1 > > /* Before VS2013, use Microsoft's nonstandard equivalent function */ > > #if (_MSC_VER < 1800) #define strtoll _strtoi64 #endif #endif > > > > and similarly for strtoull. > > > > Please check that and see if it works. > > Thanks, as you mentioned the attached works fine for me. > I think the code is also fine for the before VS2013 but I not yet tested. > > > BTW, is it possible to set up an ecpg test case to verify that this > > stuff works? > > It'd have to handle platforms without long long though, so I'm not > > sure how to deal with that. > > Yes. I was expecting that at least bigint will works fine with sqlda in > linux system but it seem did not? > Attached test patch works fine in Windows, but in Linux system the > sqlda->sqlvar[i].sqltype return ECPGt_long but not ECPGt_long_long (as > expected) when reference to bigint column. > Is this another problem in linux? Or am I wrong something? Look into the code I found that Linux also need define HAVE_LONG_LONG_INT_64 Flag in order to work with bigint and sqlda. In the following code, sqlda_dynamic_type function return ECPGt_long, If HAVE_LONG_LONG_INT_64 is not defined. ecpg/ecpg/typename.c --- sqlda_dynamic_type function ... #ifdef HAVE_LONG_LONG_INT_64 return ECPGt_long_long; #endif #ifdef HAVE_LONG_INT_64 return ECPGt_long; ... --- Is this fine if We define the above flag in pg_config.h.in like that? --- /* Define to 1 if the system has the type `long long int'. */ -#undef HAVE_LONG_LONG_INT +#define HAVE_LONG_LONG_INT 1 /* Define to 1 if `long long int' works and is 64 bits. */ -#undef HAVE_LONG_LONG_INT_64 +#define HAVE_LONG_LONG_INT_64 1 --- I am wondering that "long long int" not supported platform also using pg_config.h.in? Thanks and best regards, --- Dang Minh Huong NEC Solution Innovators, Ltd. http://www.nec-solutioninnovators.co.jp/en/
Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes: >> Attached test patch works fine in Windows, but in Linux system the >> sqlda->sqlvar[i].sqltype return ECPGt_long but not ECPGt_long_long (as >> expected) when reference to bigint column. >> Is this another problem in linux? Or am I wrong something? That would depend on what platform you're running on: if it's 64-bit then "long int" is probably 64 bits wide, making that a perfectly valid answer. AIUI, the idea in ecpg is to support "long long int" program variables if the compiler has such a type, without any particular assumptions about how wide that type is. But whether the SQL type int8 maps to long or long long is going to be platform-specific, which evidently is another hurdle to testing anything here :-( regards, tom lane
Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes: >> From: Tom Lane [mailto:tgl@sss.pgh.pa.us] >> BTW, is it possible to set up an ecpg test case to verify that this stuff >> works? >> It'd have to handle platforms without long long though, so I'm not >> sure how to deal with that. > Yes. I was expecting that at least bigint will works fine with sqlda in linux > system but it seem did not? After some thought I decided that the right fix is to add coverage for both ECPGt_long and ECPGt_long_long. Any given platform will test only one of those two code paths, but that's probably fine, because that is the only one it would use anyway. Pushed it that way; the buildfarm will soon tell us if this was a stupid idea ... regards, tom lane
I wrote: > Pushed it that way; the buildfarm will soon tell us if this was a > stupid idea ... BTW, after further digging I am suspicious that this means that we need to propagate HAVE_STRTOLL and HAVE_STRTOULL into ecpg_config.h not only pg_config.h. I'm not totally sure which compiles include just the former not the latter. I'm going to wait and see if the buildfarm is unhappy before trying to change that, though. It will help prove whether we're actually getting useful test coverage. regards, tom lane
I wrote: > BTW, after further digging I am suspicious that this means that we need > to propagate HAVE_STRTOLL and HAVE_STRTOULL into ecpg_config.h not only > pg_config.h. I'm not totally sure which compiles include just the former > not the latter. After looking closer, ecpg only examines HAVE_STRTOLL and HAVE_STRTOULL in ecpglib/data.c, which does include the main config file, so we should be good on that. > I'm going to wait and see if the buildfarm is unhappy before trying to > change that, though. It will help prove whether we're actually getting > useful test coverage. Nonetheless, all the 32-bit buildfarm critters are falling over, and the reason is now obvious: HAVE_LONG_LONG_INT isn't getting defined in the test code, because neither pg_config.h nor ecpg_config.h ever get included there. As a stopgap measure, we could stick #include <ecpg_config.h> into just that one test file. I notice however that there are more problems. In particular, sqltypes.h supposes it has access to HAVE_LONG_LONG_INT_64, which seems utterly naive. It seems like really we need <ecpg_config.h> in sqltypes.h at least, and if we don't want more bugs of the same ilk in future, it'd be wise to stick it into something that's included by all ecpg-generated code, like ecpglib.h. I am however worried about invasion of client namespace if we do that, so not quite sure what to do here. Thoughts? regards, tom lane
Hi, Thanks for working a lot in this thread. > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > I wrote: > > BTW, after further digging I am suspicious that this means that we > > need to propagate HAVE_STRTOLL and HAVE_STRTOULL into ecpg_config.h > > not only pg_config.h. I'm not totally sure which compiles include > > just the former not the latter. > > After looking closer, ecpg only examines HAVE_STRTOLL and HAVE_STRTOULL > in ecpglib/data.c, which does include the main config file, so we should > be good on that. I agreed. > > I'm going to wait and see if the buildfarm is unhappy before trying to > > change that, though. It will help prove whether we're actually > > getting useful test coverage. > > Nonetheless, all the 32-bit buildfarm critters are falling over, and the > reason is now obvious: HAVE_LONG_LONG_INT isn't getting defined in the test > code, because neither pg_config.h nor ecpg_config.h ever get included > there. > > As a stopgap measure, we could stick #include <ecpg_config.h> into just > that one test file. I notice however that there are more problems. > In particular, sqltypes.h supposes it has access to HAVE_LONG_LONG_INT_64, > which seems utterly naive. You have done it as the "hot-fix", thanks. Also thanks for fixing the failed of "Buildfarm member dromedary". # I also feel curious about it. > It seems like really we need <ecpg_config.h> in sqltypes.h at least, and > if we don't want more bugs of the same ilk in future, it'd be wise to stick > it into something that's included by all ecpg-generated code, like ecpglib.h. > I am however worried about invasion of client namespace if we do that, so > not quite sure what to do here. Thoughts? I am also inclined to the above, by the way. >> Is this another problem in linux? Or am I wrong something? > > That would depend on what platform you're running on: if it's 64-bit > then "long int" is probably 64 bits wide, making that a perfectly valid answer. Thanks, I was misunderstood. Thanks and best regards, --- Dang Minh Huong NEC Solution Innovators, Ltd. http://www.nec-solutioninnovators.co.jp/en/
Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes: >> From: Tom Lane [mailto:tgl@sss.pgh.pa.us] >> It seems like really we need <ecpg_config.h> in sqltypes.h at least, and >> if we don't want more bugs of the same ilk in future, it'd be wise to stick >> it into something that's included by all ecpg-generated code, like ecpglib.h. >> I am however worried about invasion of client namespace if we do that, so >> not quite sure what to do here. Thoughts? > I am also inclined to the above, by the way. Any other opinions about that? These are the symbols that ecpg_config.h might define, at present: HAVE_INT64 HAVE_LONG_INT_64 HAVE_LONG_LONG_INT HAVE_LONG_LONG_INT_64 ENABLE_THREAD_SAFETY This doesn't seem like a huge invasion of client namespace, but it's not entirely zero risk either. One idea is to leave the back branches as they now stand, but fix this issue in HEAD. That would leave the hazard in sqltypes.h unfixed in the back branches ... but given the lack of field complaints, a fix there could be worse than the disease. > Also thanks for fixing the failed of "Buildfarm member dromedary". > # I also feel curious about it. We still have two issues in the buildfarm: * gaur/pademelon fell over last night for lack of strtoll(). My first response to that was to disable the ecpg tests on those critters, reasoning that it couldn't possibly be worth rolling our own strtoll() in 2018. However, further digging shows that the functionality *is* available, it's just spelled __strtoll. So now I'm inclined to teach configure about that and undo the lobotomization of those animals. If we're still supporting strtoq() we can surely manage __strtoll(). * frogmouth is failing the test too, due to printing values that look like the expected 64-bit result has been truncated to 32 bits. I suspect this explains it: sqlda.pgc: In function 'dump_sqlda': sqlda.pgc:44:4: warning: unknown conversion type character 'l' in format sqlda.pgc:44:4: warning: too many arguments for format I'm guessing that the native printf() on that platform doesn't know "%lld" and is printing the value as 32 bits. This seems a bit problematic to fix. Using INT64_FORMAT isn't a solution: it's a cheat in that it's assuming that "long long" is 64 bits, and what's more, it definitely won't fix the problem on frogmouth because that build is using our own snprintf and so its value of INT64_FORMAT is calibrated to work with our code, ie, it's going to be "%lld" anyway. However ... we've got a ton of other places that use INT64_FORMAT with the native printf, eg in pgbench, and frogmouth is not producing warnings about those usages. So I'm confused about exactly what is happening there. Andrew, do you have any insight? regards, tom lane
On 05/19/2018 12:45 PM, Tom Lane wrote: > > * frogmouth is failing the test too, due to printing values that look > like the expected 64-bit result has been truncated to 32 bits. > I suspect this explains it: > > sqlda.pgc: In function 'dump_sqlda': > sqlda.pgc:44:4: warning: unknown conversion type character 'l' in format > sqlda.pgc:44:4: warning: too many arguments for format > > I'm guessing that the native printf() on that platform doesn't know > "%lld" and is printing the value as 32 bits. This seems a bit > problematic to fix. Using INT64_FORMAT isn't a solution: it's a cheat > in that it's assuming that "long long" is 64 bits, and what's more, > it definitely won't fix the problem on frogmouth because that build is > using our own snprintf and so its value of INT64_FORMAT is calibrated > to work with our code, ie, it's going to be "%lld" anyway. > > However ... we've got a ton of other places that use INT64_FORMAT with > the native printf, eg in pgbench, and frogmouth is not producing > warnings about those usages. So I'm confused about exactly what is > happening there. Andrew, do you have any insight? > > Very occasionally ;-) A little Googling suggested that __USE_MINGW_ANSI_STDIO might help. Here's what happened $ cat testme.c #include <stdio.h> int main(int argc, char **argv) { long long int val = 1111111111111111111LL; printf("sizeof long long = %d\n", sizeof(val)); printf("val = %lld (%%lld) %I64d (%%I64d)\n",val,val); return 0; } $ gcc -Wall -o testme testme.c testme.c: In function 'main': testme.c:7:5: warning: unknown conversion type character 'l' in format testme.c:7:5: warning: too many arguments for format $ ./testme sizeof long long = 8 val = 734294471 (%lld) 3153770738837321131 (%I64d) $ gcc -Wall -D__USE_MINGW_ANSI_STDIO -o testme testme.c $ ./testme sizeof long long = 8 val = 1111111111111111111 (%lld) 1111111111111111111 (%I64d) So maybe we just need to define this on XP/mingw (shouldn't be necessary on anything later). I don't know what other effects it might have, though. Perhaps there is some other flag or define that has the same effect that we use in compiling pgbench etc that isn't used by ecpg? Now recall that this animal is on serious life support. It's on an unsupported OS, none of the animals are building the master branch because of the huge pages problem, and the compiler here is pretty much unsupported also. AFAIK the Msys people now use compilers from the mingw-w64 project exclusively, and they probably do the right thing. Not sure how much more effort I should put in here. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 05/19/2018 12:45 PM, Tom Lane wrote: >> However ... we've got a ton of other places that use INT64_FORMAT with >> the native printf, eg in pgbench, and frogmouth is not producing >> warnings about those usages. So I'm confused about exactly what is >> happening there. Andrew, do you have any insight? > Very occasionally ;-) > A little Googling suggested that __USE_MINGW_ANSI_STDIO might help. > Here's what happened > $ gcc -Wall -o testme testme.c > testme.c: In function 'main': > testme.c:7:5: warning: unknown conversion type character 'l' in format > testme.c:7:5: warning: too many arguments for format > $ ./testme > sizeof long long = 8 > val = 734294471 (%lld) 3153770738837321131 (%I64d) Yeah, this agrees with what we're seeing in the ecpg test failures, both as to the warning and the wrong run-time answer. What remains unexplained is why we don't see the same compile-time warning for uses of printf("%lld") elsewhere in the build. > $ gcc -Wall -D__USE_MINGW_ANSI_STDIO -o testme testme.c [ works as expected ] > So maybe we just need to define this on XP/mingw (shouldn't be necessary > on anything later). I don't know what other effects it might have, > though. Perhaps there is some other flag or define that has the same > effect that we use in compiling pgbench etc that isn't used by ecpg? Might be worth trying. As I mentioned in <13103.1526749980@sss.pgh.pa.us>, it seems like it's time to jettison any pretense of support for non-C99-compliant spellings of "%lld". It'd be good to know whether __USE_MINGW_ANSI_STDIO works for that purpose on ancient MinGW. > Now recall that this animal is on serious life support. ... > Not sure how much more effort I should put in here. I couldn't blame you for just deciding to skip the ecpg tests on this critter. regards, tom lane
On 05/19/2018 06:49 PM, Tom Lane wrote: > >> So maybe we just need to define this on XP/mingw (shouldn't be necessary >> on anything later). I don't know what other effects it might have, >> though. Perhaps there is some other flag or define that has the same >> effect that we use in compiling pgbench etc that isn't used by ecpg? > Might be worth trying. As I mentioned in > <13103.1526749980@sss.pgh.pa.us>, it seems like it's time to jettison > any pretense of support for non-C99-compliant spellings of "%lld". > It'd be good to know whether __USE_MINGW_ANSI_STDIO works for that > purpose on ancient MinGW. Is anybody actually using the I64 stuff any more? I guess we can look at the config.logs of recent buildfarm builds. > >> Now recall that this animal is on serious life support. ... >> Not sure how much more effort I should put in here. > I couldn't blame you for just deciding to skip the ecpg tests on > this critter. > > Well, I think we can make a little effort here. I think we just need to put something like this somewhere before we include stdio.h: /* add this define on XP only, not needed elsewhere */ #ifdef WIN32_WINNT #if WIN32_WINNT < 0x0600 #define __USE_MINGW_ANSI_STDIO 1 #endif #endif cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 05/19/2018 06:49 PM, Tom Lane wrote: >> Might be worth trying. As I mentioned in >> <13103.1526749980@sss.pgh.pa.us>, it seems like it's time to jettison >> any pretense of support for non-C99-compliant spellings of "%lld". >> It'd be good to know whether __USE_MINGW_ANSI_STDIO works for that >> purpose on ancient MinGW. > Is anybody actually using the I64 stuff any more? I guess we can look at > the config.logs of recent buildfarm builds. Yeah, I already did. Every active member is reporting "ll" or "l" for INT64_MODIFIER. In some cases such as frogmouth, I believe this is because we're overriding the initial result for the benefit of our replacement snprintf --- but nonetheless, we're not using I64. The info I've been able to find on the web suggests that in all post-XP-or-so Windows versions, printf does accept "ll". > Well, I think we can make a little effort here. I think we just need to > put something like this somewhere before we include stdio.h: > /* add this define on XP only, not needed elsewhere */ > #ifdef WIN32_WINNT > #if WIN32_WINNT < 0x0600 > #define __USE_MINGW_ANSI_STDIO 1 > #endif > #endif I'd be inclined to just stick -D__USE_MINGW_ANSI_STDIO=1 into frogmouth's configuration. Given that we've already tossed support for XP overboard in HEAD, it would seem a bit silly to put it back in this way. regards, tom lane
On 05/19/2018 07:31 PM, Tom Lane wrote: > >> /* add this define on XP only, not needed elsewhere */ >> #ifdef WIN32_WINNT >> #if WIN32_WINNT < 0x0600 >> #define __USE_MINGW_ANSI_STDIO 1 >> #endif >> #endif > I'd be inclined to just stick -D__USE_MINGW_ANSI_STDIO=1 into > frogmouth's configuration. Given that we've already tossed > support for XP overboard in HEAD, it would seem a bit silly > to put it back in this way. > > Good point. Doing that now. cheers andrew
On 05/19/2018 07:33 PM, Andrew Dunstan wrote: > > > On 05/19/2018 07:31 PM, Tom Lane wrote: >> >>> /* add this define on XP only, not needed elsewhere */ >>> #ifdef WIN32_WINNT >>> #if WIN32_WINNT < 0x0600 >>> #define __USE_MINGW_ANSI_STDIO 1 >>> #endif >>> #endif >> I'd be inclined to just stick -D__USE_MINGW_ANSI_STDIO=1 into >> frogmouth's configuration. Given that we've already tossed >> support for XP overboard in HEAD, it would seem a bit silly >> to put it back in this way. >> >> > > > > Good point. Doing that now. > Well that seems to have crashed and burned badly. I'm just going to disable ecpg checks on this animal as suggested upthread. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > Well that seems to have crashed and burned badly. I'm just going to > disable ecpg checks on this animal as suggested upthread. Hmm ... this might be too much of a coincidence, but I can't help noticing that the places that are going south with -D__USE_MINGW_ANSI_STDIO are pretty nearly the same ones I just pointed to in https://www.postgresql.org/message-id/21670.1526769114@sss.pgh.pa.us as using "%lf". I'd supposed that that was mostly compulsive neatnik-ism, but is it possible that mingw's "ansi stdio" library is actually sensitive to that? regards, tom lane
On 05/20/2018 12:12 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> Well that seems to have crashed and burned badly. I'm just going to >> disable ecpg checks on this animal as suggested upthread. > Hmm ... this might be too much of a coincidence, but I can't help noticing > that the places that are going south with -D__USE_MINGW_ANSI_STDIO are > pretty nearly the same ones I just pointed to in > https://www.postgresql.org/message-id/21670.1526769114@sss.pgh.pa.us > as using "%lf". I'd supposed that that was mostly compulsive neatnik-ism, > but is it possible that mingw's "ansi stdio" library is actually > sensitive to that? > > Yeah, it sure is. With that applied ecpg-check actually passes on frogmouth. If you apply it to all the live branches I'll re-enable the tests. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 05/20/2018 12:12 AM, Tom Lane wrote: >> Hmm ... this might be too much of a coincidence, but I can't help noticing >> that the places that are going south with -D__USE_MINGW_ANSI_STDIO are >> pretty nearly the same ones I just pointed to in >> https://www.postgresql.org/message-id/21670.1526769114@sss.pgh.pa.us >> as using "%lf". I'd supposed that that was mostly compulsive neatnik-ism, >> but is it possible that mingw's "ansi stdio" library is actually >> sensitive to that? > Yeah, it sure is. With that applied ecpg-check actually passes on > frogmouth. If you apply it to all the live branches I'll re-enable the > tests. Huh. I'd have laid long odds that I was just being anal-retentive ... but sometimes it pays. Will push the fix later today. regards, tom lane