List:Commits« Previous MessageNext Message »
From:lars-erik.bjork Date:February 20 2009 12:14pm
Subject:bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:3030) Bug#23692
View as plain text  
#At file:///home/lb200670/mysql/23692/ based on revid:vvaintroub@stripped

 3030 lars-erik.bjork@stripped	2009-02-20
      This is a patch for bug#23692 (Falcon: searches fail if data is 0x00)
      
      The solution is to append a pad key to the upper 
      bound search key, if its last character is equal
      to or greater than the pad character. This is done
      In order to make it position after all values with 
      trailing characters lower than the pad character.
      
      For fields with a collation registered
      [if (field->collation)], there is no efficient way
      of checking if the last character is equal or greater
      than the pad character, without iterating through the
      entire key from the beginning.
      
      I have discussed this with Alexander Barkov who suggests
      to always pad the upper bound search key in these cases,
      and to pad it to the length of the key, instead of 
      appending just a single character. This way I can use the
      existing cs->coll->strnxfrm function.
      
      In the other cases, I have checked on the last byte and
      appended 0x20 if the byte was >= 0x20.
      
      I have also added one more parameter to the makeKey methods
      to say that this is a highKey.
modified:
  storage/falcon/Collation.h
  storage/falcon/CollationCaseless.cpp
  storage/falcon/CollationCaseless.h
  storage/falcon/CollationUnknown.cpp
  storage/falcon/CollationUnknown.h
  storage/falcon/Index.cpp
  storage/falcon/Index.h
  storage/falcon/MySQLCollation.cpp
  storage/falcon/MySQLCollation.h
  storage/falcon/NBitmap.cpp
  storage/falcon/NInSelectBitmap.cpp
  storage/falcon/StorageDatabase.cpp
  storage/falcon/StorageDatabase.h
  storage/falcon/StorageTable.cpp
  storage/falcon/ValueSet.cpp
  storage/falcon/ha_falcon.cpp

per-file messages:
  storage/falcon/Collation.h
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/CollationCaseless.cpp
    Added one more parameter to makeKey() to say if this is a highKey.
    
    Check the last byte of the key to see if a pad byte should be appended.
  storage/falcon/CollationCaseless.h
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/CollationUnknown.cpp
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/CollationUnknown.h
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/Index.cpp
    Added a parameter to Index::makeKey to say if this is a highKey. Restructured to code a little to make it more readable. For fields of the Falcon type String, Varchar and Char, and that has no collation registered, I check if a pad byte of 0x20 should be appended to the highKey
  storage/falcon/Index.h
    Added a parameter to the Index::makeKey methods to say if this is a highKey
  storage/falcon/MySQLCollation.cpp
    If this is a high key, we pad with space instead of zero. Since there is no efficient way of doing it, we don't check the final character of the key.
  storage/falcon/MySQLCollation.h
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/NBitmap.cpp
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/NInSelectBitmap.cpp
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/StorageDatabase.cpp
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/StorageDatabase.h
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/StorageTable.cpp
    If this is a highKey (upper bound serach key), we pass 'true' as the final parameter to StorageDatabase::makeKey
  storage/falcon/ValueSet.cpp
    Added one more parameter to makeKey() to say if this is a highKey
  storage/falcon/ha_falcon.cpp
    Add a wrapper function that calls strnxfrm with MY_STRXFRM_PAD_WITH_SPACE instead of 0 as the final argument
=== modified file 'storage/falcon/Collation.h'
--- a/storage/falcon/Collation.h	2007-11-27 20:07:30 +0000
+++ b/storage/falcon/Collation.h	2009-02-20 12:14:41 +0000
@@ -33,7 +33,7 @@ public:
 	Collation();
 
 	virtual int		compare (Value *value1, Value *value2) = 0;
-	virtual int		makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength) = 0;
+	virtual int		makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey) = 0;
 	virtual bool	starting (const char *string1, const char *string2) = 0;
 	virtual bool	like (const char *string, const char *pattern) = 0;
 	virtual char	getPadChar(void) = 0;

=== modified file 'storage/falcon/CollationCaseless.cpp'
--- a/storage/falcon/CollationCaseless.cpp	2008-10-31 15:42:42 +0000
+++ b/storage/falcon/CollationCaseless.cpp	2009-02-20 12:14:41 +0000
@@ -88,7 +88,7 @@ int CollationCaseless::compare(Value *va
 	return 0;
 }
 
-int CollationCaseless::makeKey(Value *value, IndexKey *key, int partialKey, int maxKeyLength)
+int CollationCaseless::makeKey(Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey)
 {
 	int l = value->getString (sizeof(key->key), (char*) key->key);
 	
@@ -103,6 +103,14 @@ int CollationCaseless::makeKey(Value *va
 	for (int n = 0; n < l; ++n)
 		p [n] = caseTable [p [n]];
 
+	// If this is a highKey, append 0x20 (pad char) if the final byte 
+	// >= 0x20. This is done when creating an upper bound
+	// search key to make it position after all values with
+	// trailing characters between 0x00 and the pad character
+
+	if (highKey && l > 0 && l < maxKeyLength && p[l-1] >= 0x20)
+		p[l++] = 0x20;
+
 	key->keyLength = l;
 
 	return l;

=== modified file 'storage/falcon/CollationCaseless.h'
--- a/storage/falcon/CollationCaseless.h	2007-11-27 20:07:30 +0000
+++ b/storage/falcon/CollationCaseless.h	2009-02-20 12:14:41 +0000
@@ -32,7 +32,7 @@ public:
 	virtual bool		like(const char * string, const char * pattern);
 	virtual bool		starting (const char *string1, const char *string2);
 	virtual const char	*getName();
-	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength);
+	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey);
 	virtual int			compare (Value *value1, Value *value2);
 	virtual char		getPadChar(void);
 	virtual int			truncate(Value *value, int partialLength);

=== modified file 'storage/falcon/CollationUnknown.cpp'
--- a/storage/falcon/CollationUnknown.cpp	2007-11-27 20:07:30 +0000
+++ b/storage/falcon/CollationUnknown.cpp	2009-02-20 12:14:41 +0000
@@ -39,12 +39,12 @@ int CollationUnknown::compare(Value *val
 	return collation->compare(value1, value2);
 }
 
-int CollationUnknown::makeKey(Value *value, IndexKey *key, int partialKey, int maxKeyLength)
+int CollationUnknown::makeKey(Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey)
 {
 	if (!collation)
 		getCollation();
 	
-	return collation->makeKey(value, key, partialKey, maxKeyLength);
+	return collation->makeKey(value, key, partialKey, maxKeyLength, highKey);
 }
 
 const char* CollationUnknown::getName()

=== modified file 'storage/falcon/CollationUnknown.h'
--- a/storage/falcon/CollationUnknown.h	2007-11-27 20:07:30 +0000
+++ b/storage/falcon/CollationUnknown.h	2009-02-20 12:14:41 +0000
@@ -28,7 +28,7 @@ public:
 	virtual ~CollationUnknown(void);
 	
 	virtual int			compare (Value *value1, Value *value2);
-	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength);
+	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey);
 	virtual const char	*getName ();
 	virtual bool		starting (const char *string1, const char *string2);
 	virtual bool		like (const char *string, const char *pattern);

=== modified file 'storage/falcon/Index.cpp'
--- a/storage/falcon/Index.cpp	2009-01-26 17:34:37 +0000
+++ b/storage/falcon/Index.cpp	2009-02-20 12:14:41 +0000
@@ -289,7 +289,7 @@ DeferredIndex *Index::getDeferredIndex(T
 }
 
 
-void Index::makeKey(Field *field, Value *value, int segment, IndexKey *indexKey)
+void Index::makeKey(Field *field, Value *value, int segment, IndexKey *indexKey, bool highKey)
 {
 	if (damaged)
 		damageCheck();
@@ -309,26 +309,40 @@ void Index::makeKey(Field *field, Value 
 
 			if (field->collation)
 				{
-				field->collation->makeKey(value, indexKey, partialLength, database->getMaxKeyLength());
-				
-				return;
+				field->collation->makeKey(value, indexKey, partialLength, database->getMaxKeyLength(), highKey);
 				}
+			else
+				{
+				UCHAR *key = indexKey->key;
 
-			UCHAR *key = indexKey->key;
-			int l = value->getString(sizeof(indexKey->key), (char*) indexKey->key);
-			
-			if (partialLength && partialLength < l)
-				l = partialLength;
-
-			UCHAR *q = key + l;
+				int l = value->getString(sizeof(indexKey->key), (char*) indexKey->key);
+				
+				if (partialLength && partialLength < l)
+					l = partialLength;
+				
+				UCHAR *q = key + l;
+				
+				while (q > key && q[-1] == ' ')
+					--q;
+				
+				indexKey->keyLength = (int) (q - key);
 
-			while (q > key && q[-1] == ' ')
-				--q;
+				// If this is a highKey, append 0x20 (pad char) if the final byte 
+				// >= 0x20. This is done when creating an upper bound
+				// search key to make it position after all values with
+				// trailing characters between 0x00 and the pad character
 
-			indexKey->keyLength = (int) (q - key);
+				uint &klen = indexKey->keyLength;
+				
+				if (highKey && (klen > 0 && klen < MAX_PHYSICAL_KEY_LENGTH) 
+					&& indexKey->key[klen - 1] >= 0x20)
+					{
+					indexKey->key[klen++] = 0x20;
+					}
 
-			return ;
+				}
 			}
+			break;
 
 		case Timestamp:
 		case Date:
@@ -344,7 +358,7 @@ void Index::makeKey(Field *field, Value 
 		}
 }
 
-void Index::makeKey(int count, Value **values, IndexKey *indexKey)
+void Index::makeKey(int count, Value **values, IndexKey *indexKey, bool highKey)
 {
 	if (damaged)
 		damageCheck();
@@ -360,7 +374,7 @@ void Index::makeKey(int count, Value **v
 
 	if (numberFields == 1)
 		{
-		makeKey(fields[0], values[0], 0, indexKey);
+		makeKey(fields[0], values[0], 0, indexKey, highKey);
 		
 		return;
 		}
@@ -375,7 +389,7 @@ void Index::makeKey(int count, Value **v
 
 		
 		IndexKey tempKey(this);
-		makeKey(field, values[n], n, &tempKey);
+		makeKey(field, values[n], n, &tempKey, false);
 		int length = tempKey.keyLength;
 		UCHAR *t = tempKey.key;
 
@@ -636,7 +650,7 @@ void Index::makeKey(Record * record, Ind
 		record->getValue (field->id, value);
 		}
 		
-	makeKey (numberFields, values, key);
+	makeKey (numberFields, values, key, false);
 }
 
 void Index::garbageCollect(Record * leaving, Record * staying, Transaction *transaction, bool quiet)

=== modified file 'storage/falcon/Index.h'
--- a/storage/falcon/Index.h	2008-08-19 03:33:01 +0000
+++ b/storage/falcon/Index.h	2009-02-20 12:14:41 +0000
@@ -107,8 +107,8 @@ public:
 	void		checkMaxKeyLength(void);
 
 	void		makeKey (Record *record, IndexKey *key);
-	void		makeKey (int count, Value **values, IndexKey *key);
-	void		makeKey (Field *field, Value *value, int segment, IndexKey *key);
+	void		makeKey (int count, Value **values, IndexKey *key, bool highKey);
+	void		makeKey (Field *field, Value *value, int segment, IndexKey *key, bool highKey);
 
 	void		detachDeferredIndex(DeferredIndex *deferredIndex);
 	UCHAR		getPadByte(void);

=== modified file 'storage/falcon/MySQLCollation.cpp'
--- a/storage/falcon/MySQLCollation.cpp	2008-12-04 11:00:12 +0000
+++ b/storage/falcon/MySQLCollation.cpp	2009-02-20 12:14:41 +0000
@@ -50,7 +50,7 @@ int MySQLCollation::compare (Value *valu
 	return falcon_strnncoll(charset, string1, len1, string2, len2, false);
 }
 
-int MySQLCollation::makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength)
+int MySQLCollation::makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey)
 {
 	if (partialKey > maxKeyLength )
 		partialKey = maxKeyLength;
@@ -69,7 +69,24 @@ int MySQLCollation::makeKey (Value *valu
 	if (!isBinary)
 		srcLen = computeKeyLength(srcLen, temp, padChar, minSortChar);
 
-	int len = falcon_strnxfrm(charset, (char*) key->key, MAX_PHYSICAL_KEY_LENGTH, partialKey ? partialKey / falcon_get_mbmaxlen(charset) : maxKeyLength, temp, srcLen);
+
+	int len = 0;
+
+	// If this is a highKey, append the pad char if the final 
+	// character is >= the pad char. There is no efficient way 
+	// of finding and checking the final character for every 
+	// character set, but it should be correct to pad the rest 
+	// of the key with the pad character anyway. This is done 
+	// when creating an upper bound search key to make it position
+	// after all values with trailing characters lower than the 
+	// pad character.
+
+	if (highKey)
+		len = falcon_strnxfrm_space_pad(charset, (char*) key->key, MAX_PHYSICAL_KEY_LENGTH, 
+										partialKey ? partialKey / falcon_get_mbmaxlen(charset) : maxKeyLength, temp, srcLen);
+	else
+		len = falcon_strnxfrm(charset, (char*) key->key, MAX_PHYSICAL_KEY_LENGTH, 
+							  partialKey ? partialKey / falcon_get_mbmaxlen(charset) : maxKeyLength, temp, srcLen);
 	key->keyLength = len;
 	
 	return len;

=== modified file 'storage/falcon/MySQLCollation.h'
--- a/storage/falcon/MySQLCollation.h	2007-11-27 20:07:30 +0000
+++ b/storage/falcon/MySQLCollation.h	2009-02-20 12:14:41 +0000
@@ -25,6 +25,10 @@ extern int falcon_strnxfrm (void *cs, 
 							const char *dst, uint dstlen, int nweights,
 							const char *src, uint srclen);
 
+extern int falcon_strnxfrm_space_pad (void *cs, 
+							const char *dst, uint dstlen, int nweights,
+							const char *src, uint srclen);
+
 extern char falcon_get_pad_char (void *cs);
 extern int falcon_cs_is_binary (void *cs);
 extern unsigned int falcon_get_mbmaxlen (void *cs);
@@ -49,7 +53,7 @@ public:
 	~MySQLCollation(void);
 
 	virtual int			compare (Value *value1, Value *value2);
-	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength);
+	virtual int			makeKey (Value *value, IndexKey *key, int partialKey, int maxKeyLength, bool highKey);
 	virtual const char	*getName ();
 	virtual bool		starting (const char *string1, const char *string2);
 	virtual bool		like (const char *string, const char *pattern);

=== modified file 'storage/falcon/NBitmap.cpp'
--- a/storage/falcon/NBitmap.cpp	2007-10-11 12:57:56 +0000
+++ b/storage/falcon/NBitmap.cpp	2009-02-20 12:14:41 +0000
@@ -105,7 +105,7 @@ void NBitmap::makeKey(Statement *stateme
 			break;
 			}
 
-	index->makeKey(nodeCount, values, indexKey);
+	index->makeKey(nodeCount, values, indexKey, false);
 }
 
 NNode* NBitmap::clone()

=== modified file 'storage/falcon/NInSelectBitmap.cpp'
--- a/storage/falcon/NInSelectBitmap.cpp	2007-09-20 15:44:25 +0000
+++ b/storage/falcon/NInSelectBitmap.cpp	2009-02-20 12:14:41 +0000
@@ -68,7 +68,7 @@ Bitmap* NInSelectBitmap::evalInversion(S
 		{
 		IndexKey indexKey(index);
 		Value *value = resultSet->getValue (1);
-		index->makeKey (1, &value, &indexKey);
+		index->makeKey (1, &value, &indexKey, false);
 		Bitmap *bits = index->scanIndex (&indexKey, &indexKey, index->numberFields > 1, statement->transaction, NULL);
 
 		if (bits)

=== modified file 'storage/falcon/StorageDatabase.cpp'
--- a/storage/falcon/StorageDatabase.cpp	2009-01-20 08:09:02 +0000
+++ b/storage/falcon/StorageDatabase.cpp	2009-02-20 12:14:41 +0000
@@ -799,7 +799,7 @@ IndexWalker* StorageDatabase::indexPosit
 								storageConnection->connection->getTransaction());
 }
 
-int StorageDatabase::makeKey(StorageIndexDesc* indexDesc, const UCHAR* key, int keyLength, StorageKey* storageKey)
+int StorageDatabase::makeKey(StorageIndexDesc* indexDesc, const UCHAR* key, int keyLength, StorageKey* storageKey, bool highKey)
 {
 	int segmentNumber = 0;
 	Value vals [MAX_KEY_SEGMENTS];
@@ -827,7 +827,7 @@ int StorageDatabase::makeKey(StorageInde
 			p += len;
 			}
 
-		index->makeKey(segmentNumber, values, &storageKey->indexKey);
+		index->makeKey(segmentNumber, values, &storageKey->indexKey, highKey);
 		storageKey->numberSegments = segmentNumber;
 		
 		return 0;

=== modified file 'storage/falcon/StorageDatabase.h'
--- a/storage/falcon/StorageDatabase.h	2009-01-07 08:12:08 +0000
+++ b/storage/falcon/StorageDatabase.h	2009-02-20 12:14:41 +0000
@@ -64,7 +64,7 @@ public:
 	int					renameTable(StorageConnection* storageConnection, Table* table, const char* newName, const char *schemaName);
 	Bitmap*				indexScan(Index* index, StorageKey *lower, StorageKey *upper, int searchFlags, StorageConnection* storageConnection, Bitmap *bitmap);
 	IndexWalker*		indexPosition(Index* index, StorageKey* lower, StorageKey* upper, int searchFlags, StorageConnection* storageConnection);
-	int					makeKey(StorageIndexDesc* index, const UCHAR* key, int keyLength, StorageKey* storageKey);
+	int					makeKey(StorageIndexDesc* index, const UCHAR* key, int keyLength, StorageKey* storageKey, bool highKey);
 	int					storeBlob(Connection* connection, Table* table, StorageBlob* blob);
 	void				getBlob(Table* table, int recordNumber, StorageBlob* blob);
 	void				addRef(void);

=== modified file 'storage/falcon/StorageTable.cpp'
--- a/storage/falcon/StorageTable.cpp	2009-02-12 20:22:40 +0000
+++ b/storage/falcon/StorageTable.cpp	2009-02-20 12:14:41 +0000
@@ -291,7 +291,7 @@ int StorageTable::setIndexBound(const un
 	if (which & LowerBound)
 		{
 		lowerBound = &lowerKey;
-		ret = storageDatabase->makeKey(currentIndex, key, keyLength, lowerBound);
+		ret = storageDatabase->makeKey(currentIndex, key, keyLength, lowerBound, false);
 		
 		if (ret)
 			return ret;
@@ -302,7 +302,14 @@ int StorageTable::setIndexBound(const un
 	else if (which & UpperBound)
 		{
 		upperBound = &upperKey;
-		ret = storageDatabase->makeKey(currentIndex, key, keyLength, upperBound);
+
+		// Because this is an upper bound search key, we pass 
+		// 'true' as the final argument to makeKey. This way a 
+		// pad byte (if needed) will be appended to the key to 
+		// ensure that values below the pad character sort correctly.
+		// See bug#23692 for a more detailed explanation
+
+		ret = storageDatabase->makeKey(currentIndex, key, keyLength, upperBound, true);
 
 		if (ret)
 			return ret;

=== modified file 'storage/falcon/ValueSet.cpp'
--- a/storage/falcon/ValueSet.cpp	2007-09-20 15:44:25 +0000
+++ b/storage/falcon/ValueSet.cpp	2009-02-20 12:14:41 +0000
@@ -43,7 +43,7 @@ void ValueSet::addValue(Value *value)
 	//UCHAR key[MAX_KEY_LENGTH];
 	//int len = index->makeKey(field, value, key);
 	IndexKey indexKey(index);
-	index->makeKey(field, value, 0, &indexKey);
+	index->makeKey(field, value, 0, &indexKey, false);
 }
 
 bool ValueSet::hasValue(Value *value)
@@ -51,7 +51,7 @@ bool ValueSet::hasValue(Value *value)
 	//UCHAR key[MAX_KEY_LENGTH];
 	//int len = index->makeKey(field, value, key);
 	IndexKey indexKey(index);
-	index->makeKey(field, value, 0, &indexKey);
+	index->makeKey(field, value, 0, &indexKey, false);
 
 	return true;
 }

=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp	2009-02-11 18:23:17 +0000
+++ b/storage/falcon/ha_falcon.cpp	2009-02-20 12:14:41 +0000
@@ -299,6 +299,16 @@ int falcon_strnxfrm (void *cs,
 	                              (uchar *) src, srclen, 0);
 }
 
+int falcon_strnxfrm_space_pad (void *cs,
+                     const char *dst, uint dstlen, int nweights,
+                     const char *src, uint srclen)
+{
+	CHARSET_INFO *charset = (CHARSET_INFO*) cs;
+
+	return (int)charset->coll->strnxfrm(charset, (uchar *) dst, dstlen, nweights,
+	                              (uchar *) src, srclen, MY_STRXFRM_PAD_WITH_SPACE);
+}
+
 char falcon_get_pad_char (void *cs)
 {
 	return (char) ((CHARSET_INFO*) cs)->pad_char;

Thread
bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:3030) Bug#23692lars-erik.bjork20 Feb