Обсуждение: pgsql: Prevent infinity and NaN in jsonb/plperl transform
Prevent infinity and NaN in jsonb/plperl transform jsonb uses numeric internally, and numeric can store NaN, but that is not allowed by jsonb on input, so we shouldn't store it. Also prevent infinity to get a consistent error message. (numeric input would reject infinity anyway.) Reported-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/e348e7ae5727a6da8678036d748e5c5af7deb6c9 Modified Files -------------- contrib/jsonb_plperl/expected/jsonb_plperl.out | 24 ++++++++++++++++++++++-- contrib/jsonb_plperl/expected/jsonb_plperlu.out | 24 ++++++++++++++++++++++-- contrib/jsonb_plperl/jsonb_plperl.c | 16 ++++++++++++++-- contrib/jsonb_plperl/sql/jsonb_plperl.sql | 22 ++++++++++++++++++++++ contrib/jsonb_plperl/sql/jsonb_plperlu.sql | 22 ++++++++++++++++++++++ 5 files changed, 102 insertions(+), 6 deletions(-)
Peter Eisentraut <peter_e@gmx.net> writes:
> Prevent infinity and NaN in jsonb/plperl transform
So this isn't working on Windows:
contrib/jsonb_plperl/jsonb_plperl.c(226): warning C4013: 'isnan' undefined; assuming extern returning int
[c:\pgbuildfarm\pgbuildroot\HEAD\pgsql.build\jsonb_plperl.vcxproj]
...
.\Release\jsonb_plperl\jsonb_plperl.dll : fatal error LNK1120: 1 unresolved externals
[c:\pgbuildfarm\pgbuildroot\HEAD\pgsql.build\jsonb_plperl.vcxproj]
The reason seems to be this kluge in plperl.h:
/* stop perl headers from hijacking stdio and other stuff on Windows */
#ifdef WIN32
#define WIN32IO_IS_STDIO
/*
* isnan is defined in both the perl and mingw headers. We don't use it,
* so this just clears up the compile warning.
*/
#ifdef isnan
#undef isnan
#endif
#endif /* WIN32 */
Looks like the half-life of that hack just expired. What shall
we do about it?
If you don't have an immediate solution, please revert this commit
temporarily, as it's preventing me from making any further progress
on the reattach-failure investigation.
regards, tom lane
On 4/30/18 13:56, Tom Lane wrote: > So this isn't working on Windows: > > contrib/jsonb_plperl/jsonb_plperl.c(226): warning C4013: 'isnan' undefined; assuming extern returning int [c:\pgbuildfarm\pgbuildroot\HEAD\pgsql.build\jsonb_plperl.vcxproj] > ... > .\Release\jsonb_plperl\jsonb_plperl.dll : fatal error LNK1120: 1 unresolved externals [c:\pgbuildfarm\pgbuildroot\HEAD\pgsql.build\jsonb_plperl.vcxproj] > > The reason seems to be this kluge in plperl.h: > > /* stop perl headers from hijacking stdio and other stuff on Windows */ > #ifdef WIN32 > #define WIN32IO_IS_STDIO > /* > * isnan is defined in both the perl and mingw headers. We don't use it, > * so this just clears up the compile warning. > */ > #ifdef isnan > #undef isnan > #endif > #endif /* WIN32 */ > > Looks like the half-life of that hack just expired. What shall > we do about it? I have removed this for now. If it's really just about a compiler warning, then we can find a different workaround later. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 4/30/18 13:56, Tom Lane wrote:
>> So this isn't working on Windows: ...
>> The reason seems to be this kluge in plperl.h:
>>
>> /* stop perl headers from hijacking stdio and other stuff on Windows */
>> #ifdef WIN32
>> #define WIN32IO_IS_STDIO
>> /*
>> * isnan is defined in both the perl and mingw headers. We don't use it,
>> * so this just clears up the compile warning.
>> */
>> #ifdef isnan
>> #undef isnan
>> #endif
>> #endif /* WIN32 */
>>
>> Looks like the half-life of that hack just expired. What shall
>> we do about it?
> I have removed this for now. If it's really just about a compiler
> warning, then we can find a different workaround later.
I was just about to propose exactly that fix. This undef'ing of isnan
traces back to Andrew's 2006 commit ea73a78b0. I think perhaps that's not
necessary with any still-supported version of Perl; I see no evidence on
my machines that any Perl header #defines isnan. There is this bit:
#ifdef UNDER_CE
int isnan(double d);
#endif
which would cause trouble if isnan is a macro, but we can hope that
that doesn't apply to us.
regards, tom lane
On 04/30/2018 02:41 PM, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> On 4/30/18 13:56, Tom Lane wrote: >>> So this isn't working on Windows: ... >>> The reason seems to be this kluge in plperl.h: >>> >>> /* stop perl headers from hijacking stdio and other stuff on Windows */ >>> #ifdef WIN32 >>> #define WIN32IO_IS_STDIO >>> /* >>> * isnan is defined in both the perl and mingw headers. We don't use it, >>> * so this just clears up the compile warning. >>> */ >>> #ifdef isnan >>> #undef isnan >>> #endif >>> #endif /* WIN32 */ >>> >>> Looks like the half-life of that hack just expired. What shall >>> we do about it? >> I have removed this for now. If it's really just about a compiler >> warning, then we can find a different workaround later. > I was just about to propose exactly that fix. This undef'ing of isnan > traces back to Andrew's 2006 commit ea73a78b0. I think perhaps that's not > necessary with any still-supported version of Perl; I see no evidence on > my machines that any Perl header #defines isnan. There is this bit: > > #ifdef UNDER_CE > int isnan(double d); > #endif > > which would cause trouble if isnan is a macro, but we can hope that > that doesn't apply to us. > > Fingers crossed we'll be OK. Looking at the perl headers on a Windows machine I'm doing some other stuff on, the #define is protected by an #ifndef. I'm always happy when my hacks can be removed :-) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> Fingers crossed we'll be OK. Looking at the perl headers on a Windows
> machine I'm doing some other stuff on, the #define is protected by an
> #ifndef. I'm always happy when my hacks can be removed :-)
Well, the early returns on that are promising, but we have another
problem: prairiedog, jacana, and probably other old BF members are
failing the new regression test cases. It appears that what the test
script assumes will produce a NaN or Inf float doesn't work in 5.8.x
perl:
$ perl -e "print 0 + 'NaN'"
0
$ perl -e "print 0 + 'Inf'"
0
I googled a bit and found these recommendations on stackoverflow:
my $inf = 9**9**9;
my $neginf = -9**9**9;
my $nan = -sin(9**9**9);
These do seem to produce the desired results, at least on the
couple of Perl versions I checked, including 5.8.3.
regards, tom lane
I wrote:
> I googled a bit and found these recommendations on stackoverflow:
> my $inf = 9**9**9;
> my $neginf = -9**9**9;
> my $nan = -sin(9**9**9);
> These do seem to produce the desired results, at least on the
> couple of Perl versions I checked, including 5.8.3.
Nope, buildfarm shows that still doesn't work everywhere.
If we had a transform function that converted SQL float8 directly
to Perl NV, we could use that to produce NVs containing inf/nan.
But that would be a pretty ridiculous amount of test scaffolding
to create to test what, in the end, is two lines of very
straightforward and unlikely-to-break code.
My recommendation is just to drop these test cases. Getting them
to work on all old Perl versions is more trouble than they're worth.
regards, tom lane
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 04/30/2018 02:41 PM, Tom Lane wrote:
>> I was just about to propose exactly that fix. This undef'ing of isnan
>> traces back to Andrew's 2006 commit ea73a78b0. I think perhaps that's not
>> necessary with any still-supported version of Perl; I see no evidence on
>> my machines that any Perl header #defines isnan. ...
> Fingers crossed we'll be OK. Looking at the perl headers on a Windows
> machine I'm doing some other stuff on, the #define is protected by an
> #ifndef. I'm always happy when my hacks can be removed :-)
All of the Windows machines except bowerbird and hamerkop have now
reported in on this change, and none of them are showing any warnings
about isnan. I'll be surprised if bowerbird does, since it's an
intermediate version of MSVC between ones that already reported.
hamerkop seems to be the oldest MSVC version we have, so conceivably
it'll issue a warning; but unless it fails outright I'd be inclined to
ignore it.
regards, tom lane