Thread: BUG #1608: integer negative limit in plpgsql function arguments

The following bug has been logged online:

Bug reference:      1608
Logged by:          Paul
Email address:      paul@allinea.com
PostgreSQL version: 8.0.2
Operating system:   Gentoo and Fedora Core 3
Description:        integer negative limit in plpgsql function arguments
Details:

The script below best sums up the problem (and the work around).  The
question is: should I use that for all integers being put into a function?

8<--------------------------------------------

create table test (
    test_id integer
);

insert into test (test_id) values (-2147483648);

create function print_test_id (integer) returns integer
    AS '
DECLARE
    tmp ALIAS FOR $1;
    val integer;
BEGIN
    select into val test_id from test where test_id = tmp;
    return val;
END;
'
    LANGUAGE plpgsql;

-- this doesn't work (and I think it should!)
SELECT print_test_id(-2147483648);

-- this is the workaround
SELECT print_test_id((-2147483648)::int);

-------------------------------------------->8

"Paul" <paul@allinea.com> writes:
> -- this doesn't work (and I think it should!)
> SELECT print_test_id(-2147483648);

"2147483648" isn't an integer constant; it's int8, and therefore so
is the result of the minus operator.  Sorry, this isn't going to
change.

            regards, tom lane

Paul Edwards <paul@allinea.com> writes:
> Also, just as an experiment I tried the minimum limit for bigint (see
> attached file).  It seems that I do not need to cast for negative limit
> which is inconsistent since 9223372036854775808 is not a bigint (when
> -9223372036854775808 is).  Therefore the type wasn't necessarily
> determined before the unary operator.

Really?  [ tries it ... then reads some code ... ]

You're right, we do cheat a little on negative numeric constants --- I
had forgotten about the doNegate() hack in gram.y.  We could conceivably
fix it to cheat some more.  Specifically it looks like make_const() in
parse_node.c could check for the possibility that a T_Float fits in INT4
--- which would happen only for the case of -2147483648, since any
smaller absolute value would have been T_Integer to start with.

This also brings up the thought that maybe the T_Integer case should
create an INT2 rather than INT4 Const if the value is small enough.
I'm fairly hesitant to do that though because it would be a significant
change in behavior, possibly breaking apps that don't have a problem
now.  (IIRC we experimented with such a change some years back and saw
widespread failures in the regression tests, for example.)

However changing the behavior only for -2147483648 seems like a
relatively safe thing to do.

Thoughts, objections anyone?

            regards, tom lane

I wrote:
> You're right, we do cheat a little on negative numeric constants --- I
> had forgotten about the doNegate() hack in gram.y.  We could conceivably
> fix it to cheat some more.  Specifically it looks like make_const() in
> parse_node.c could check for the possibility that a T_Float fits in INT4
> --- which would happen only for the case of -2147483648, since any
> smaller absolute value would have been T_Integer to start with.

I've applied the attached patch to CVS HEAD.  I'm not going to risk
back-patching this, but feel free to use the patch locally if it's
important to you.

            regards, tom lane

*** src/backend/parser/parse_node.c.orig    Fri Dec 31 17:45:55 2004
--- src/backend/parser/parse_node.c    Sat Apr 23 14:28:03 2005
***************
*** 304,314 ****
              /* could be an oversize integer as well as a float ... */
              if (scanint8(strVal(value), true, &val64))
              {
!                 val = Int64GetDatum(val64);

!                 typeid = INT8OID;
!                 typelen = sizeof(int64);
!                 typebyval = false;        /* XXX might change someday */
              }
              else
              {
--- 304,331 ----
              /* could be an oversize integer as well as a float ... */
              if (scanint8(strVal(value), true, &val64))
              {
!                 /*
!                  * It might actually fit in int32. Probably only INT_MIN can
!                  * occur, but we'll code the test generally just to be sure.
!                  */
!                 int32    val32 = (int32) val64;

!                 if (val64 == (int64) val32)
!                 {
!                     val = Int32GetDatum(val32);
!
!                     typeid = INT4OID;
!                     typelen = sizeof(int32);
!                     typebyval = true;
!                 }
!                 else
!                 {
!                     val = Int64GetDatum(val64);
!
!                     typeid = INT8OID;
!                     typelen = sizeof(int64);
!                     typebyval = false;        /* XXX might change someday */
!                 }
              }
              else
              {