New improved version of bpcharin() may have got the truncation case

right, but it failed to get the padding case right.

This was obscured by subsequent application of bpchar() in all but one
regression test case, and that one didn't fail in an obvious way ---
trailing blanks are hard to see.  Add another test case to make it
more obvious if it breaks again.

diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 467a5cf7de3975223533af95ce35804d539949ac..cdd76afb55d06040cb4d5c3b5276bd35221d19cd 100644 (file)
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/utils/adt/varchar.c,v 1.78 2001/05/21 16:54:46 petere Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/utils/adt/varchar.c,v 1.79 2001/06/01 17:49:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -65,10 +65,8 @@ Datum
 bpcharin(PG_FUNCTION_ARGS)
 {
    char       *s = PG_GETARG_CSTRING(0);
-
 #ifdef NOT_USED
    Oid         typelem = PG_GETARG_OID(1);
-
 #endif
    int32       atttypmod = PG_GETARG_INT32(2);
    BpChar     *result;
@@ -77,45 +75,46 @@ bpcharin(PG_FUNCTION_ARGS)
    int         i;
 
    len = strlen(s);
-   maxlen = atttypmod - VARHDRSZ;
 
-   if (atttypmod >= (int32) VARHDRSZ && len > maxlen)
+   /* If typmod is -1 (or invalid), use the actual string length */
+   if (atttypmod < (int32) VARHDRSZ)
+       maxlen = len;
+   else
+       maxlen = atttypmod - VARHDRSZ;
+
+   if (len > maxlen)
    {
+       /* Verify that extra characters are spaces, and clip them off */
 #ifdef MULTIBYTE
        size_t mbmaxlen = pg_mbcliplen(s, len, maxlen);
 
        if (strspn(s + mbmaxlen, " ") == len - mbmaxlen)
            len = mbmaxlen;
+       else
+           elog(ERROR, "value too long for type character(%d)", maxlen);
+       Assert(len <= maxlen);
 #else
        if (strspn(s + maxlen, " ") == len - maxlen)
-           /* clip extra spaces */
            len = maxlen;
-#endif
        else
            elog(ERROR, "value too long for type character(%d)", maxlen);
+#endif
    }
-   else
-       /* If typmod is -1 (or invalid), use the actual string length */
-       maxlen = len;
 
    result = palloc(maxlen + VARHDRSZ);
    VARATT_SIZEP(result) = maxlen + VARHDRSZ;
    r = VARDATA(result);
-   for (i = 0; i < len; i++, r++, s++)
-   {
-       *r = *s;
-       if (*r == '\0')
-           break;
-   }
-
-#ifdef CYR_RECODE
-   convertstr(VARDATA(result), len, 0);
-#endif
+   for (i = 0; i < len; i++)
+       *r++ = *s++;
 
    /* blank pad the string if necessary */
    for (; i < maxlen; i++)
        *r++ = ' ';
 
+#ifdef CYR_RECODE
+   convertstr(VARDATA(result), len, 0);
+#endif
+
    PG_RETURN_BPCHAR_P(result);
 }
 
@@ -162,11 +161,12 @@ bpchar(PG_FUNCTION_ARGS)
 
    len = VARSIZE(source);
    /* No work if typmod is invalid or supplied data matches it already */
-   if (len < (int32) VARHDRSZ || len == maxlen)
+   if (maxlen < (int32) VARHDRSZ || len == maxlen)
        PG_RETURN_BPCHAR_P(source);
 
    if (len > maxlen)
    {
+       /* Verify that extra characters are spaces, and clip them off */
 #ifdef MULTIBYTE
        size_t      maxmblen;
 
@@ -179,13 +179,13 @@ bpchar(PG_FUNCTION_ARGS)
                     maxlen - VARHDRSZ);
 
        len = maxmblen;
+       Assert(len <= maxlen);
 #else
        for (i = maxlen - VARHDRSZ; i < len - VARHDRSZ; i++)
            if (*(VARDATA(source) + i) != ' ')
                elog(ERROR, "value too long for type character(%d)",
                     maxlen - VARHDRSZ);
 
-       /* clip extra spaces */
        len = maxlen;
 #endif
    }
@@ -196,7 +196,7 @@ bpchar(PG_FUNCTION_ARGS)
    VARATT_SIZEP(result) = maxlen;
    r = VARDATA(result);
 
-   for (i = 0; (i < maxlen - VARHDRSZ) && (i < len - VARHDRSZ); i++)
+   for (i = 0; i < len - VARHDRSZ; i++)
        *r++ = *s++;
 
    /* blank pad the string if necessary */
@@ -340,10 +340,8 @@ Datum
 varcharin(PG_FUNCTION_ARGS)
 {
    char       *s = PG_GETARG_CSTRING(0);
-
 #ifdef NOT_USED
    Oid         typelem = PG_GETARG_OID(1);
-
 #endif
    int32       atttypmod = PG_GETARG_INT32(2);
    VarChar    *result;
@@ -354,6 +352,7 @@ varcharin(PG_FUNCTION_ARGS)
 
    if (atttypmod >= (int32) VARHDRSZ && len > maxlen)
    {
+       /* Verify that extra characters are spaces, and clip them off */
 #ifdef MULTIBYTE
        size_t mbmaxlen = pg_mbcliplen(s, len, maxlen);
 
@@ -361,7 +360,6 @@ varcharin(PG_FUNCTION_ARGS)
            len = mbmaxlen;
 #else
        if (strspn(s + maxlen, " ") == len - maxlen)
-           /* clip extra spaces */
            len = maxlen;
 #endif
        else
@@ -419,6 +417,7 @@ varchar(PG_FUNCTION_ARGS)
    int         i;
 
    len = VARSIZE(source);
+   /* No work if typmod is invalid or supplied data fits it already */
    if (maxlen < (int32) VARHDRSZ || len <= maxlen)
        PG_RETURN_VARCHAR_P(source);
 

diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index 7562a25fdca5399ae0c4b745243c76a38b4e1afe..42df7c06df2e698b8357878b0053684be81b6ef8 100644 (file)
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -481,10 +481,16 @@ SELECT text 'text' || ' and unknown' AS "Concat text to unknown type";
  text and unknown
 (1 row)
 
+SELECT char(20) 'characters' || 'and text' AS "Concat char to unknown type";
+ Concat char to unknown type  
+------------------------------
+ characters          and text
+(1 row)
+
 SELECT text 'text' || char(20) ' and characters' AS "Concat text to char";
- Concat text to char 
----------------------
- text and characters
+   Concat text to char    
+--------------------------
+ text and characters     
 (1 row)
 
 SELECT text 'text' || varchar ' and varchar' AS "Concat text to varchar";

diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index 56510f83dddc9eee78dadaae42adfbc6340d8933..b7f214f4d89c30c573cdb7c13a9149f302f5d5d9 100644 (file)
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -160,6 +160,8 @@ SELECT 'unknown' || ' and unknown' AS "Concat unknown types";
 
 SELECT text 'text' || ' and unknown' AS "Concat text to unknown type";
 
+SELECT char(20) 'characters' || 'and text' AS "Concat char to unknown type";
+
 SELECT text 'text' || char(20) ' and characters' AS "Concat text to char";
 
 SELECT text 'text' || varchar ' and varchar' AS "Concat text to varchar";