Обсуждение: 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

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/

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/

> 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