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

Hello, the work is finished.
Version 4 of the patch is attached to this message.
- Add rough description of the algorithm as comment to  pg_utf8_increment() and pg_eucjp_increment().
- Fixed a bug of pg_utf8_increment() found while  working. pg_(utf8|eucjp)_increment are retested on whole valid  code points to be properly handled.
- The comment previously pointed out as being wrong in grammar  is left untouched. I'm sorry to bother you with my poor  English.


At Tue, 11 Oct 2011 16:55:00 +0900 (JST), Kyotaro HORIGUCHI  wrote 
> >   One thing I still think it would be useful to add,
> > though, is some comments to pg_utf8_increment() and
> > pg_eucjp_increment() describing the algorithm being used.  Can you
> > take a crack at that?
> 
>  Yes I'll do it in a day or two.
Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 8ceea82..59f8c37 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5664,6 +5664,19 @@ 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 a palloc'd string * in the form of a Const node; else return NULL.
@@ -5702,6 +5715,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
@@ -5763,29 +5777,33 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)		}	}
+	if (datatype == BYTEAOID)
+		charincfunc = &byte_increment;
+	else
+		charincfunc = pg_database_encoding_character_incrementer();
+	while (len > 0)	{
-		unsigned char *lastchar = (unsigned char *) (workstr + len - 1);
-		unsigned char savelastchar = *lastchar;
+		int charlen = 1;
+		unsigned char *lastchar;
+		Const	   *workstr_const;
+		
+		if (datatype != BYTEAOID)
+			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.
+		 * Try to generate a larger string by incrementing the last byte or
+		 * character.		 */
-		while (*lastchar < (unsigned char) 255)
-		{
-			Const	   *workstr_const;
-
-			(*lastchar)++;
-			if (datatype != BYTEAOID)
-			{
-				/* do not generate invalid encoding sequences */
-				if (!pg_verifymbstr(workstr, len, true))
-					continue;
-				workstr_const = string_to_const(workstr, datatype);
-			}
-			else
+		if (charincfunc(lastchar, charlen))
+		{
+			if (datatype == BYTEAOID)				workstr_const = string_to_bytea_const(workstr, len);
+			else
+				workstr_const = string_to_const(workstr, datatype);			if (DatumGetBool(FunctionCall2Coll(ltproc,											   collation,
@@ -5804,20 +5822,10 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)			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 byte.		 */
-		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;	}	/* Failed... */
diff --git a/src/backend/utils/mb/wchar.c b/src/backend/utils/mb/wchar.c
index f23732f..a213636 100644
--- a/src/backend/utils/mb/wchar.c
+++ b/src/backend/utils/mb/wchar.c
@@ -1336,6 +1336,264 @@ 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.
+ * -------------------------------------------------------------------
+ */
+
+#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;
+}
+ 
+/*
+ * Calculate the valid UTF-8 byte sequence that is the smallest of the
+ * greater than the given valid UTF-8 byte sequence for single
+ * character. The word `valid' here means that pg_utf8_islegal() says
+ * that it is valid.  pg_utf8_increment does this roughly with the
+ * following algorithm.
+ * 
+ *  - If the length of the sequence is 1, the byte is between 0x01 and
+ *    0x7f. It is simplly incremented if less than 0x7f.
+ *
+ *  - Otherwise the bytes after the first one ranges between 0x80 and
+ *    0xbf.  The sequence is incremented from the last byte and the
+ *    carry is propagated up to the first byte.
+ *
+ * Only the UTF-8 byte sequences corresponding to the following
+ * unicode point which have no character allocated make this function
+ * to fail in spite of its validity.
+ *
+ * U+0007F, U+007FF, U+0FFFF, U+10FFFF
+ * 
+ */
+static bool
+pg_utf8_increment(unsigned char *charptr, int length)
+{
+ 	unsigned char a;
+ 	unsigned char bak[4];
+	unsigned char limit;
+ 
+ 	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];
+			switch (*charptr)
+			{
+			case 0xED:
+				limit = 0x9F;
+				break;
+
+			case 0xF4:
+				limit = 0x8F;
+				break;
+
+			default:
+				limit = 0xBF;
+
+			}
+
+			if (a < limit)
+			{
+ 				charptr[1]++;
+ 				break;
+ 			}
+
+ 			charptr[1] = 0x80;
+ 			/* FALL THRU */
+ 		case 1:
+			bak[0] = *charptr;
+ 			a = *charptr;
+ 			if (a == 0x7F || a == 0xDF || a == 0xEF || a == 0xF4)
+			{
+				/* 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;
+}
+ 
+/* Calculate the valid EUC-JP byte sequence that is the smallest of
+ * the greater than the given valid EUC-JP byte sequence for single
+ * character. The word `valid' here means that pg_eucjp_verifier()
+ * syas that it is valid. This function does this roughly with the
+ * following algorithm.
+ *
+ *  - If the sequence starts with SS2(0x8e), it must be a two-byte
+ *    sequence representing JIS X 0201 characters with the second byte
+ *    ranges between 0xa1 and 0xde. It is simplly incremented at the
+ *    second byte if less than 0xde, and otherwise rewrite whole the
+ *    sequence to 0xa1 0xa1 as the result.
+ *
+ *  - If the sequence starts with SS3(0x8f), it must be a three-byte
+ *    sequence which the last two bytes ranges between 0xa1 and 0xfe.
+ *    The bytes are incremented from the last within the range and
+ *    propagate the carry up to the first byte.
+ *
+ *  - If the sequence starts with the values other than the aboves and
+ *    its MSB is set, it must be a two-byte sequence representing JIS
+ *    X 0208 characters with both bytes ranges between 0xa1 and 0xfe.
+ *    The bytes are incremented from the latter within the range and
+ *    propagate the carry up to the first byte.
+ *
+ *  - Otherwise the sequence is consists of single byte representing
+ *    ASCII characters. It is incremented up to 0x7f.
+ *    
+ * Only three EUC-JP byte sequences shown below which has no character
+ * allocated make this function to fail in spite of its validity.
+ *
+ * 0x7f, 0xfe 0xfe, 0x8f 0xfe 0xfe
+ *
+ */
+static bool
+pg_eucjp_increment(unsigned char *charptr, int length)
+{
+ 	unsigned char bak[3];
+ 	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 the same order as enum pg_enc (in mb/pg_wchar.h) *-------------------------------------------------------------------
@@ -1459,6 +1717,25 @@ 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) (const unsigned 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(const char *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);extern int	SetClientEncoding(int encoding);