pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
6+ messages / 3 participants
[nested] [flat]
* BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-29 22:35 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2021-07-29 22:35 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: kjs@teews.com
The following bug has been logged on the website:
Bug reference: 17128
Logged by: Kevin Sweet
Email address: kjs@teews.com
PostgreSQL version: 13.2
Operating system: RHEL6, RHEL7, Solaris11
Description:
I submitted a similar report to the documentation mailing list so you can
decide whether to update the documentation or the code.
The PGTYPESnumeric_to_int function deems -2147483648 to be invalid even
though it is a perfectly valid 32-bit integer because the code compares to
-INT_MAX which resolves to -2147483647 on the Fedora/Red Hat and Solaris
versions I have available to check against.
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 7266e229a4..0e89f23c73 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -8666,7 +8666,7 @@ int dectoint(decimal *np, int *ip);
Note that the ECPG implementation differs from the
<productname>Informix</productname>
implementation. <productname>Informix</productname> limits an
integer to the range from -32767 to
32767, while the limits in the ECPG implementation depend on the
- architecture (<literal>-INT_MAX .. INT_MAX</literal>).
+ architecture (<literal>(-INT_MAX - 1) .. INT_MAX</literal>).
</para>
</listitem>
</varlistentry>
diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c
b/src/interfaces/ecpg/pgtypeslib/numeric.c
index 060fad7867..4e6c9910dc 100644
--- a/src/interfaces/ecpg/pgtypeslib/numeric.c
+++ b/src/interfaces/ecpg/pgtypeslib/numeric.c
@@ -1505,7 +1505,7 @@ PGTYPESnumeric_to_int(numeric *nv, int *ip)
if ((i = PGTYPESnumeric_to_long(nv, &l)) != 0)
return i;
- if (l < -INT_MAX || l > INT_MAX)
+ if (l < (-INT_MAX - 1) || l > INT_MAX)
{
errno = PGTYPES_NUM_OVERFLOW;
return -1;
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-30 15:05 John Naylor <john.naylor@enterprisedb.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 6+ messages in thread
From: John Naylor @ 2021-07-30 15:05 UTC (permalink / raw)
To: kjs@teews.com; pgsql-bugs@lists.postgresql.org
On Fri, Jul 30, 2021 at 7:53 AM PG Bug reporting form <
noreply@postgresql.org> wrote:
> The PGTYPESnumeric_to_int function deems -2147483648 to be invalid even
> though it is a perfectly valid 32-bit integer because the code compares to
> -INT_MAX which resolves to -2147483647 on the Fedora/Red Hat and Solaris
> versions I have available to check against.
> - if (l < -INT_MAX || l > INT_MAX)
> + if (l < (-INT_MAX - 1) || l > INT_MAX)
Yeah, that looks like it should be INT_MIN instead. I'll see about making
that happen. Thanks for the report!
--
John Naylor
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-30 15:14 Tom Lane <tgl@sss.pgh.pa.us>
parent: John Naylor <john.naylor@enterprisedb.com>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2021-07-30 15:14 UTC (permalink / raw)
To: John Naylor <john.naylor@enterprisedb.com>; +Cc: kjs@teews.com; pgsql-bugs@lists.postgresql.org
John Naylor <john.naylor@enterprisedb.com> writes:
> On Fri, Jul 30, 2021 at 7:53 AM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>> - if (l < -INT_MAX || l > INT_MAX)
>> + if (l < (-INT_MAX - 1) || l > INT_MAX)
> Yeah, that looks like it should be INT_MIN instead. I'll see about making
> that happen. Thanks for the report!
The whole stanza perhaps ought to be within
#if SIZEOF_LONG > SIZEOF_INT
otherwise some compilers will bleat about useless tests.
(I looked at PGTYPESnumeric_to_long and it seems like it will do
the right thing for 32-bit long, assuming strtol does.)
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-30 17:56 John Naylor <john.naylor@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: John Naylor @ 2021-07-30 17:56 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: kjs@teews.com; pgsql-bugs@lists.postgresql.org
On Fri, Jul 30, 2021 at 11:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> John Naylor <john.naylor@enterprisedb.com> writes:
> > On Fri, Jul 30, 2021 at 7:53 AM PG Bug reporting form <
> > noreply@postgresql.org> wrote:
> >> - if (l < -INT_MAX || l > INT_MAX)
> >> + if (l < (-INT_MAX - 1) || l > INT_MAX)
>
> > Yeah, that looks like it should be INT_MIN instead. I'll see about
making
> > that happen. Thanks for the report!
>
> The whole stanza perhaps ought to be within
>
> #if SIZEOF_LONG > SIZEOF_INT
>
> otherwise some compilers will bleat about useless tests.
Here's a draft fix for master, with regression tests. It will need a bit of
massaging for the back branches -- the problem goes back at least to 9.6.
--
John Naylor
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Fix-range-check-in-ECPG-numeric-to-int-conversion.patch (application/x-patch, 5.7K) download
From 9f4791f7a660171b6f57b1b336d6412d21ebd549 Mon Sep 17 00:00:00 2001
From: John Naylor <john.naylor@postgresql.org>
Date: Fri, 30 Jul 2021 13:50:23 -0400
Subject: [PATCH v1] Fix range check in ECPG numeric to int conversion
The previous coding guarded against -INT_MAX instead of INT_MIN,
leading to -2147483648 being rejected as out of range.
Per bug #17128 from Kevin Sweet
Discussion: https://www.postgresql.org/message-id/flat/17128-55a8a879727a3e3a%40postgresql.org
Reviewed-by: Tom Lane
Backpatch to all supported branches
---
doc/src/sgml/ecpg.sgml | 2 +-
src/interfaces/ecpg/pgtypeslib/numeric.c | 7 ++++++-
.../ecpg/test/expected/pgtypeslib-num_test.c | 21 ++++++++++++++-----
.../test/expected/pgtypeslib-num_test.stderr | 2 +-
.../test/expected/pgtypeslib-num_test.stdout | 2 ++
.../ecpg/test/pgtypeslib/num_test.pgc | 13 +++++++++++-
6 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 9d5505cb84..04d4a6bdb2 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -8881,7 +8881,7 @@ int dectoint(decimal *np, int *ip);
Note that the ECPG implementation differs from the <productname>Informix</productname>
implementation. <productname>Informix</productname> limits an integer to the range from -32767 to
32767, while the limits in the ECPG implementation depend on the
- architecture (<literal>-INT_MAX .. INT_MAX</literal>).
+ architecture (<literal>INT_MIN .. INT_MAX</literal>).
</para>
</listitem>
</varlistentry>
diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c b/src/interfaces/ecpg/pgtypeslib/numeric.c
index 060fad7867..dbe4eb21e2 100644
--- a/src/interfaces/ecpg/pgtypeslib/numeric.c
+++ b/src/interfaces/ecpg/pgtypeslib/numeric.c
@@ -1505,12 +1505,17 @@ PGTYPESnumeric_to_int(numeric *nv, int *ip)
if ((i = PGTYPESnumeric_to_long(nv, &l)) != 0)
return i;
- if (l < -INT_MAX || l > INT_MAX)
+/* silence compilers that might complain about useless tests */
+#if SIZEOF_LONG > SIZEOF_INT
+
+ if (l < INT_MIN || l > INT_MAX)
{
errno = PGTYPES_NUM_OVERFLOW;
return -1;
}
+#endif
+
*ip = (int) l;
return 0;
}
diff --git a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.c b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.c
index bf312549b4..ddb78600e3 100644
--- a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.c
+++ b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.c
@@ -75,7 +75,7 @@ main(void)
double d;
long l1, l2;
- int i;
+ int i, min, max;
ECPGdebug(1, stderr);
/* exec sql whenever sqlerror do sqlprint ( ) ; */
@@ -174,17 +174,28 @@ if (sqlca.sqlcode < 0) sqlprint ( );}
PGTYPESnumeric_free(value2);
PGTYPESnumeric_free(res);
+ /* check conversion of numeric to int */
+ value1 = PGTYPESnumeric_from_asc("-2147483648", NULL);
+ PGTYPESnumeric_to_int(value1, &min);
+ printf("min int = %d\n", min);
+ PGTYPESnumeric_free(value1);
+
+ value2 = PGTYPESnumeric_from_asc("2147483647", NULL);
+ PGTYPESnumeric_to_int(value2, &max);
+ printf("max int = %d\n", max);
+ PGTYPESnumeric_free(value2);
+
{ ECPGtrans(__LINE__, NULL, "rollback");
-#line 90 "num_test.pgc"
+#line 101 "num_test.pgc"
if (sqlca.sqlcode < 0) sqlprint ( );}
-#line 90 "num_test.pgc"
+#line 101 "num_test.pgc"
{ ECPGdisconnect(__LINE__, "CURRENT");
-#line 91 "num_test.pgc"
+#line 102 "num_test.pgc"
if (sqlca.sqlcode < 0) sqlprint ( );}
-#line 91 "num_test.pgc"
+#line 102 "num_test.pgc"
return 0;
diff --git a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stderr b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stderr
index a7d125402a..10cfd91cec 100644
--- a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stderr
+++ b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stderr
@@ -26,7 +26,7 @@
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_get_data on line 61: RESULT: 2369.7000000 offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGtrans on line 90: action "rollback"; connection "ecpg1_regression"
+[NO_PID]: ECPGtrans on line 101: action "rollback"; connection "ecpg1_regression"
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection ecpg1_regression closed
[NO_PID]: sqlca: code: 0, state: 00000
diff --git a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stdout b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stdout
index 204c3cf6c0..a47a044c3a 100644
--- a/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stdout
+++ b/src/interfaces/ecpg/test/expected/pgtypeslib-num_test.stdout
@@ -4,3 +4,5 @@ sub = 2369.7
mul = 13306998429.873000000
div = 1330699.84298730000 1.3307e+06
to long(0) = 20000000 14
+min int = -2147483648
+max int = 2147483647
diff --git a/src/interfaces/ecpg/test/pgtypeslib/num_test.pgc b/src/interfaces/ecpg/test/pgtypeslib/num_test.pgc
index 254aeb4129..c6c2858b81 100644
--- a/src/interfaces/ecpg/test/pgtypeslib/num_test.pgc
+++ b/src/interfaces/ecpg/test/pgtypeslib/num_test.pgc
@@ -19,7 +19,7 @@ main(void)
exec sql end declare section;
double d;
long l1, l2;
- int i;
+ int i, min, max;
ECPGdebug(1, stderr);
exec sql whenever sqlerror do sqlprint();
@@ -87,6 +87,17 @@ main(void)
PGTYPESnumeric_free(value2);
PGTYPESnumeric_free(res);
+ /* check conversion of numeric to int */
+ value1 = PGTYPESnumeric_from_asc("-2147483648", NULL);
+ PGTYPESnumeric_to_int(value1, &min);
+ printf("min int = %d\n", min);
+ PGTYPESnumeric_free(value1);
+
+ value2 = PGTYPESnumeric_from_asc("2147483647", NULL);
+ PGTYPESnumeric_to_int(value2, &max);
+ printf("max int = %d\n", max);
+ PGTYPESnumeric_free(value2);
+
exec sql rollback;
exec sql disconnect;
--
2.31.1
* Re: BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-30 19:22 Tom Lane <tgl@sss.pgh.pa.us>
parent: John Naylor <john.naylor@enterprisedb.com>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2021-07-30 19:22 UTC (permalink / raw)
To: John Naylor <john.naylor@enterprisedb.com>; +Cc: kjs@teews.com; pgsql-bugs@lists.postgresql.org
John Naylor <john.naylor@enterprisedb.com> writes:
> Here's a draft fix for master, with regression tests.
Passes an eyeball check, though I didn't run the test.
> It will need a bit of
> massaging for the back branches -- the problem goes back at least to 9.6.
I'm sure it's quite ancient ... ecpg hasn't changed much in a Long Time.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented
@ 2021-07-30 20:44 John Naylor <john.naylor@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 6+ messages in thread
From: John Naylor @ 2021-07-30 20:44 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: kjs@teews.com; pgsql-bugs@lists.postgresql.org
On Fri, Jul 30, 2021 at 3:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> John Naylor <john.naylor@enterprisedb.com> writes:
> > Here's a draft fix for master, with regression tests.
>
> Passes an eyeball check, though I didn't run the test.
Pushed, thanks for looking! I'll keep an eye on the build farm.
--
John Naylor
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2021-07-30 20:44 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 22:35 BUG #17128: minimum numeric 'integer' is -2147483647 not -2147483648 as documented PG Bug reporting form <noreply@postgresql.org>
2021-07-30 15:05 ` John Naylor <john.naylor@enterprisedb.com>
2021-07-30 15:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-07-30 17:56 ` John Naylor <john.naylor@enterprisedb.com>
2021-07-30 19:22 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-07-30 20:44 ` John Naylor <john.naylor@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox