Re: [v9.2] make_greater_string() does not return a string in some cases

This is new version of make_greater_string patch.

1. wchar.c:1532 pg_wchar_table: Restore the pg_wchar_table.

2. wchar.c:1371 pg_utf8_increment: Remove dangerous memcpy, but  one memcpy is left because it's safe. Remove code
checkafter  increment.
 

3. wchar.c:1429 pg_eucjp_increment: Remove dangerous  memcpy. Remove code check after increment. Minor bug fix.

4. wchar.c:1654 pg_database_encoding_character_incrementer:  Select increment function by switch-select.



horiguchi.kyotaro> This is rebased patch of `Allow encoding specific character
horiguchi.kyotaro> incrementer'(https://commitfest.postgresql.org/action/patch_view?id=602).

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 3e84679..593dba6 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5653,6 +5653,18 @@ pattern_selectivity(Const *patt, Pattern_Type ptype)/*
+ * This function is "character increment" function for bytea used in
+ * make_greater_string() that has same interface with pg_wchar_tbl.charinc.
+ */
+static bool byte_increment(unsigned char *ptr, int len)
+{
+    if (*ptr >= 255) return false;
+
+    (*ptr)++;
+    return true;
+}
+
+/* * Try to generate a string greater than the given string or any * string it is a prefix of.  If successful, return
apalloc'd string * in the form of a Const node; else return NULL.
 
@@ -5691,6 +5703,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)    int
len;   Datum        cmpstr;    text       *cmptxt = NULL;
 
+    character_incrementer charincfunc;    /*     * Get a modifiable copy of the prefix string in C-string format, and
set
@@ -5752,27 +5765,38 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)        }    }
+    if (datatype != BYTEAOID)
+        charincfunc = pg_database_encoding_character_incrementer();
+    else
+        charincfunc = &byte_increment;
+    while (len > 0)    {
-        unsigned char *lastchar = (unsigned char *) (workstr + len - 1);
-        unsigned char savelastchar = *lastchar;
+        int charlen;
+        unsigned char *lastchar;
+        unsigned char savelastbyte;
+        Const       *workstr_const;
+        
+        if (datatype == BYTEAOID)
+            charlen = 1;
+        else
+            charlen = len - pg_mbcliplen(workstr, len, len - 1);
+
+        lastchar = (unsigned char *) (workstr + len - charlen);        /*
-         * Try to generate a larger string by incrementing the last byte.
+         * savelastbyte has meaning only for datatype == BYTEAOID         */
-        while (*lastchar < (unsigned char) 255)
-        {
-            Const       *workstr_const;
+        savelastbyte = *lastchar;
-            (*lastchar)++;
+        /*
+         * Try to generate a larger string by incrementing the last byte or
+         * character.
+         */
+        if (charincfunc(lastchar, charlen)) {            if (datatype != BYTEAOID)
-            {
-                /* do not generate invalid encoding sequences */
-                if (!pg_verifymbstr(workstr, len, true))
-                    continue;                workstr_const = string_to_const(workstr, datatype);
-            }            else                workstr_const = string_to_bytea_const(workstr, len);
@@ -5787,26 +5811,17 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
pfree(workstr);               return workstr_const;            }
 
-
+                        /* No good, release unusable value and try again */
pfree(DatumGetPointer(workstr_const->constvalue));           pfree(workstr_const);        }
 
-        /* restore last byte so we don't confuse pg_mbcliplen */
-        *lastchar = savelastchar;
-        /*
-         * Truncate off the last character, which might be more than 1 byte,
-         * depending on the character encoding.
+         * Truncate off the last character or restore last byte for BYTEA.         */
-        if (datatype != BYTEAOID && pg_database_encoding_max_length() > 1)
-            len = pg_mbcliplen(workstr, len, len - 1);
-        else
-            len -= 1;
-
-        if (datatype != BYTEAOID)
-            workstr[len] = '\0';
+        len -= charlen;
+        workstr[len] = (datatype != BYTEAOID ? '\0' : savelastbyte);    }    /* Failed... */
diff --git a/src/backend/utils/mb/wchar.c b/src/backend/utils/mb/wchar.c
index f23732f..d6dc717 100644
--- a/src/backend/utils/mb/wchar.c
+++ b/src/backend/utils/mb/wchar.c
@@ -1336,6 +1336,195 @@ pg_utf8_islegal(const unsigned char *source, int length)/*
*-------------------------------------------------------------------
+ * character incrementer
+ *
+ * These functions accept "charptr", a pointer to the first byte of a
+ * maybe-multibyte character. Try `increment' the character and return true if
+ * successed.  If these functions returns false, the character should be
+ * untouched.  These functions must be implemented in correspondence with
+ * verifiers, in other words, the rewrited character by this function must pass
+ * the check by pg_*_verifier() if returns true. Returning the return value of
+ * pg_*_verifier() corresponding can finnaly avoid such a inconsistency when
+ * something wrong.
+ * -------------------------------------------------------------------
+ */
+
+#ifndef FRONTEND
+static bool pg_generic_charinc(unsigned char *charptr, int len)
+{
+     unsigned char *lastchar = (unsigned char *) (charptr + len - 1);
+     unsigned char savelastchar = *lastchar;
+     const char *const_charptr = (const char *)charptr;
+ 
+     while (*lastchar < (unsigned char) 255)
+     {
+         (*lastchar)++;
+         if (!pg_verifymbstr(const_charptr, len, true))
+             continue;
+         return true;
+     }
+ 
+     *lastchar = savelastchar;
+     return false;
+}
+ 
+static bool
+pg_utf8_increment(unsigned char *charptr, int length)
+{
+     unsigned char a;
+     unsigned char bak[4];
+     bool success;
+ 
+     switch (length)
+     {
+         default:
+             /* reject lengths 5 and 6 for now */
+             return false;
+         case 4:
+            bak[3] = charptr[3];
+             a = charptr[3];
+             if (a < 0xBF)
+             {
+                 charptr[3]++;
+                 break;
+             }
+             charptr[3] = 0x80;
+             /* FALL THRU */
+         case 3:
+            bak[2] = charptr[2];
+             a = charptr[2];
+             if (a < 0xBF)
+             {
+                 charptr[2]++;
+                 break;
+             }
+             charptr[2] = 0x80;
+             /* FALL THRU */
+         case 2:
+            bak[1] = charptr[1];
+             a = charptr[1];
+             if ((*charptr == 0xed && a < 0x9F) || a < 0xBF)
+             {
+                 charptr[1]++;
+                 break;
+             }
+             charptr[1] = 0x80;
+             /* FALL THRU */
+         case 1:
+            bak[0] = *charptr;
+             a = *charptr;
+             if (a == 0x7F || a == 0xDF || a == 0xEF || a == 0xF7) {
+                /* Rewinding modified bytes and return fail. length is
+                 * confirmed to be between 1 and 4 here. */
+                memcpy(charptr, bak, length);
+                 return false;
+             }
+             charptr[0]++;
+             break;
+     }
+     
+     return true;
+}
+ 
+static bool
+pg_eucjp_increment(unsigned char *charptr, int length)
+{
+     unsigned char bak[3];
+     bool success;
+     unsigned char c1, c2;
+     signed int i;
+ 
+     c1 = *charptr;
+ 
+     switch (c1)
+     {
+         case SS2:    /* JIS X 0201 */
+             if (length != 2) return false;
+ 
+             c2 = charptr[1];
+ 
+             if (c2 > 0xde)
+                 charptr[0] = charptr[1] = 0xa1;
+             else if (c2 < 0xa1)
+                 charptr[1] = 0xa1;
+             else
+                 charptr[1]++;
+ 
+             break;
+ 
+         case SS3:    /* JIS X 0212 */
+             if (length != 3) return false;
+ 
+             for (i = 2 ; i > 0 ; i--)
+             {
+                bak[i] = charptr[i];
+                 c2 = charptr[i];
+                 if (c2 < 0xa1)
+                 {
+                     charptr[i] = 0xa1;
+                     return true;
+                 }
+                 else if (c2 < 0xfe)
+                 {
+                     charptr[i]++;
+                     break;
+                 }
+                 charptr[i] = 0xa1;
+             }
+ 
+ 
+             if (i == 0)      /* Out of 3-byte code region */
+             {
+                charptr[1] = bak[1];
+                charptr[2] = bak[2];
+                 return false;
+             }
+             
+             break;
+ 
+         default:
+             if (IS_HIGHBIT_SET(c1))     /* JIS X 0208? */
+             {
+                 if (length != 2) return false;
+           
+                 for (i = 1 ; i >= 0 ; i--)    /* i must be signed */
+                 {
+                    bak[i] = charptr[i];
+                     c2 = charptr[i];
+                     if (c2 < 0xa1)
+                     {
+                         charptr[i] = 0xa1;
+                         return true;
+                     }
+                     else if (c2 < 0xfe)
+                     {
+                         charptr[i]++;
+                         break;
+                     }
+                     charptr[i] = 0xa1;
+                 }
+           
+                 if (i < 0)    /*  Out of 2 byte code region */
+                 {
+                     charptr[0] = bak[0];
+                     charptr[1] = bak[1];
+                     return false;
+                 }
+             }
+             else
+             {    /* ASCII, single byte */
+                 if (c1 > 0x7e)
+                     return false;
+                 (*charptr)++;
+             }
+     }
+   
+     return true;
+}
+#endif
+
+/*
+ *------------------------------------------------------------------- * encoding info table * XXX must be sorted by
thesame order as enum pg_enc (in mb/pg_wchar.h) *-------------------------------------------------------------------
 
@@ -1459,6 +1648,24 @@ pg_database_encoding_max_length(void)}/*
+ * give the character incrementer for the encoding for the current database
+ */
+character_incrementer
+pg_database_encoding_character_incrementer(void)
+{
+    switch (GetDatabaseEncoding()) {
+        case PG_UTF8:
+            return pg_utf8_increment;
+
+        case PG_EUC_JP:
+            return pg_eucjp_increment;
+
+        default:
+            return pg_generic_charinc;
+    }
+}
+
+/* * Verify mbstr to make sure that it is validly encoded in the current * database encoding.  Otherwise same as
pg_verify_mbstr().*/
 
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index 826c7af..728175c 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -284,6 +284,8 @@ typedef int (*mblen_converter) (const unsigned char *mbstr);typedef int (*mbdisplaylen_converter)
(constunsigned char *mbstr);
 
+typedef bool (*character_incrementer) (unsigned char *mbstr, int len);
+typedef int (*mbverifier) (const unsigned char *mbstr, int len);typedef struct
@@ -389,6 +391,7 @@ extern int pg_encoding_mbcliplen(int encoding, const char *mbstr,extern int
pg_mbcharcliplen(constchar *mbstr, int len, int imit);extern int    pg_encoding_max_length(int encoding);extern int
pg_database_encoding_max_length(void);
+extern character_incrementer pg_database_encoding_character_incrementer(void);extern int    PrepareClientEncoding(int
encoding);externint    SetClientEncoding(int encoding);