tighten input to float4/float8/oid - Mailing list pgsql-patches

This patch makes the following changes

- emit a warning when an empty string is input to the float4, float8,
and oid types, per earlier discussion on -hackers
- emit a warning when there is trailing whitespace in the input to the
oid type, per suggestion from Tom (BTW, I couldn't see anything about
this in the standard, although I may have been looking in the wrong
place; in any case, rejecting trailing whitespace seems clearly
correct behavior to me)
- update the regression tests

The first two changes are intended as a temporary measures for
backward compatibility: 7.5 will emit warnings, but 7.6 will reject
this input. That should hopefully avoid a similar situation to the
pg_atoi() change.

I invented a new SQLSTATE code for these warnings,
ERRCODE_WARNING_DEPRECATED_FEATURE. Did I do this correctly?

-Neil

Index: doc/src/sgml/errcodes.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.2
diff -c -r1.2 errcodes.sgml
*** a/doc/src/sgml/errcodes.sgml    29 Nov 2003 19:51:37 -0000    1.2
--- b/doc/src/sgml/errcodes.sgml    4 Mar 2004 02:05:49 -0000
***************
*** 95,100 ****
--- 95,104 ----
  <entry>WARNING STRING DATA RIGHT TRUNCATION</entry>
  </row>

+ <row>
+ <entry><literal>01P01</literal></entry>
+ <entry>WARNING DEPRECATED FEATURE</entry>
+ </row>

  <row>
  <entry>Class 02</entry>
Index: src/backend/utils/adt/float.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/float.c,v
retrieving revision 1.96
diff -c -r1.96 float.c
*** a/src/backend/utils/adt/float.c    29 Nov 2003 19:51:58 -0000    1.96
--- b/src/backend/utils/adt/float.c    4 Mar 2004 00:31:59 -0000
***************
*** 209,214 ****
--- 209,227 ----
      }

      /*
+      * In releases prior to 7.5, we accepted an empty string as valid
+      * input (yielding a float4 of 0). In 7.5, we accept empty
+      * strings, but emit a warning noting that the feature is
+      * deprecated. In 7.6+, the warning should be replaced by an error.
+      */
+     if (num == endptr)
+         ereport(WARNING,
+                 (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE),
+                  errmsg("deprecated input syntax for type real: \"\""),
+                  errdetail("this input will be rejected in "
+                            "a future release of PostgreSQL")));
+
+     /*
       * if we get here, we have a legal double, still need to check to see
       * if it's a legal float
       */
***************
*** 309,314 ****
--- 322,340 ----
                       errmsg("\"%s\" is out of range for type double precision", num)));
      }

+     /*
+      * In releases prior to 7.5, we accepted an empty string as valid
+      * input (yielding a float8 of 0). In 7.5, we accept empty
+      * strings, but emit a warning noting that the feature is
+      * deprecated. In 7.6+, the warning should be replaced by an error.
+      */
+     if (num == endptr)
+         ereport(WARNING,
+                 (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE),
+                  errmsg("deprecated input syntax for type double precision: \"\""),
+                  errdetail("this input will be rejected in "
+                            "a future release of PostgreSQL")));
+
      CheckFloat8Val(val);

      PG_RETURN_FLOAT8(val);
Index: src/backend/utils/adt/oid.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/oid.c,v
retrieving revision 1.54
diff -c -r1.54 oid.c
*** a/src/backend/utils/adt/oid.c    18 Feb 2004 00:01:34 -0000    1.54
--- b/src/backend/utils/adt/oid.c    4 Mar 2004 00:56:48 -0000
***************
*** 40,54 ****
       * strtoul() normally only sets ERANGE.  On some systems it also may
       * set EINVAL, which simply means it couldn't parse the input string.
       * This is handled by the second "if" consistent across platforms.
-      * Note that for historical reasons we accept an empty string as
-      * meaning 0.
       */
      if (errno && errno != ERANGE && errno != EINVAL)
          ereport(ERROR,
                  (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                   errmsg("invalid input syntax for type oid: \"%s\"",
                          s)));
!     if (endptr == s && *endptr)
          ereport(ERROR,
                  (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                   errmsg("invalid input syntax for type oid: \"%s\"",
--- 40,66 ----
       * strtoul() normally only sets ERANGE.  On some systems it also may
       * set EINVAL, which simply means it couldn't parse the input string.
       * This is handled by the second "if" consistent across platforms.
       */
      if (errno && errno != ERANGE && errno != EINVAL)
          ereport(ERROR,
                  (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                   errmsg("invalid input syntax for type oid: \"%s\"",
                          s)));
!
!     /*
!      * In releases prior to 7.5, we accepted an empty string as valid
!      * input (yielding an OID of 0). In 7.5, we accept empty strings,
!      * but emit a warning noting that the feature is deprecated. In
!      * 7.6+, the warning should be replaced by an error.
!      */
!     if (*s == '\0')
!         ereport(WARNING,
!                 (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE),
!                  errmsg("deprecated input syntax for type oid: \"\""),
!                  errdetail("this input will be rejected in "
!                            "a future release of PostgreSQL")));
!
!     if (endptr == s && *s)
          ereport(ERROR,
                  (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                   errmsg("invalid input syntax for type oid: \"%s\"",
***************
*** 66,79 ****
      }
      else
      {
!         /* allow only whitespace after number */
          while (*endptr && isspace((unsigned char) *endptr))
              endptr++;
          if (*endptr)
              ereport(ERROR,
                      (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                       errmsg("invalid input syntax for type oid: \"%s\"",
                              s)));
      }

      result = (Oid) cvt;
--- 78,111 ----
      }
      else
      {
!         bool seen_whitespace = false;
!         /* allow only whitespace after number; see below */
          while (*endptr && isspace((unsigned char) *endptr))
+         {
              endptr++;
+             seen_whitespace = true;
+         }
+
          if (*endptr)
              ereport(ERROR,
                      (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                       errmsg("invalid input syntax for type oid: \"%s\"",
                              s)));
+
+         /*
+          * In releases prior to 7.5, we allowed any number of trailing
+          * whitespace characters following the numeric portion of the
+          * OID. In 7.5, we still accept this kind of input, but issue
+          * a warning that it is deprecated. In 7.6+, we will issue an
+          * error rather than a warning.
+          */
+         if (seen_whitespace)
+             ereport(WARNING,
+                     (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE),
+                      errmsg("deprecated input syntax for type oid: \"%s\"", s),
+                      errhint("trailing whitespace in this context has been deprecated"),
+                      errdetail("this input will be rejected in "
+                                "a future release of PostgreSQL")));
      }

      result = (Oid) cvt;
Index: src/include/utils/errcodes.h
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/include/utils/errcodes.h,v
retrieving revision 1.7
diff -c -r1.7 errcodes.h
*** a/src/include/utils/errcodes.h    29 Nov 2003 22:41:15 -0000    1.7
--- b/src/include/utils/errcodes.h    4 Mar 2004 02:05:04 -0000
***************
*** 44,49 ****
--- 44,50 ----
  #define ERRCODE_WARNING_IMPLICIT_ZERO_BIT_PADDING    MAKE_SQLSTATE('0','1', '0','0','8')
  #define ERRCODE_WARNING_NULL_VALUE_ELIMINATED_IN_SET_FUNCTION    MAKE_SQLSTATE('0','1', '0','0','3')
  #define ERRCODE_WARNING_STRING_DATA_RIGHT_TRUNCATION    MAKE_SQLSTATE('0','1', '0','0','4')
+ #define ERRCODE_WARNING_DEPRECATED_FEATURE    MAKE_SQLSTATE('0','1', 'P','0','1')

  /* Class 02 - No Data --- this is also a warning class per SQL99 */
  /* (do not use this class for failure conditions!) */
Index: src/test/regress/expected/oid.out
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/expected/oid.out,v
retrieving revision 1.8
diff -c -r1.8 oid.out
*** a/src/test/regress/expected/oid.out    18 Feb 2004 00:01:34 -0000    1.8
--- b/src/test/regress/expected/oid.out    4 Mar 2004 00:35:57 -0000
***************
*** 7,13 ****
  INSERT INTO OID_TBL(f1) VALUES ('987');
  INSERT INTO OID_TBL(f1) VALUES ('-1040');
  INSERT INTO OID_TBL(f1) VALUES ('99999999');
- INSERT INTO OID_TBL(f1) VALUES ('');
  -- bad inputs
  INSERT INTO OID_TBL(f1) VALUES ('asdfasd');
  ERROR:  invalid input syntax for type oid: "asdfasd"
--- 7,12 ----
***************
*** 21,28 ****
       |        987
       | 4294966256
       |   99999999
!      |          0
! (6 rows)

  SELECT '' AS one, o.* FROM OID_TBL o WHERE o.f1 = 1234;
   one |  f1
--- 20,26 ----
       |        987
       | 4294966256
       |   99999999
! (5 rows)

  SELECT '' AS one, o.* FROM OID_TBL o WHERE o.f1 = 1234;
   one |  f1
***************
*** 37,59 ****
        |        987
        | 4294966256
        |   99999999
!       |          0
! (5 rows)

  SELECT '' AS three, o.* FROM OID_TBL o WHERE o.f1 <= '1234';
   three |  f1
  -------+------
         | 1234
         |  987
!        |    0
! (3 rows)

  SELECT '' AS two, o.* FROM OID_TBL o WHERE o.f1 < '1234';
   two | f1
  -----+-----
       | 987
!      |   0
! (2 rows)

  SELECT '' AS four, o.* FROM OID_TBL o WHERE o.f1 >= '1234';
   four |     f1
--- 35,54 ----
        |        987
        | 4294966256
        |   99999999
! (4 rows)

  SELECT '' AS three, o.* FROM OID_TBL o WHERE o.f1 <= '1234';
   three |  f1
  -------+------
         | 1234
         |  987
! (2 rows)

  SELECT '' AS two, o.* FROM OID_TBL o WHERE o.f1 < '1234';
   two | f1
  -----+-----
       | 987
! (1 row)

  SELECT '' AS four, o.* FROM OID_TBL o WHERE o.f1 >= '1234';
   four |     f1
Index: src/test/regress/sql/oid.sql
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/sql/oid.sql,v
retrieving revision 1.4
diff -c -r1.4 oid.sql
*** a/src/test/regress/sql/oid.sql    21 Nov 2000 03:23:21 -0000    1.4
--- b/src/test/regress/sql/oid.sql    4 Mar 2004 00:33:10 -0000
***************
*** 14,21 ****

  INSERT INTO OID_TBL(f1) VALUES ('99999999');

- INSERT INTO OID_TBL(f1) VALUES ('');
-
  -- bad inputs

  INSERT INTO OID_TBL(f1) VALUES ('asdfasd');
--- 14,19 ----