bzr push into mysql-6.0-falcon branch (cpowers:2813 to 2814) Bug#38041

[Christopher Powers]

 2814 Christopher Powers	2008-09-08
      Bug#38041 Bizarre errors when ALTER ADD/DROP KEY on Falcon tables
      
      - Added validation methods to detect out-of-sync condition between server and falcon
indexes
      - Don't create StorageIndexDesc objects if internal falcon index does not exist
      - Enabled HA_ADD/DROP_UNIQUE_INDEX
      - Rebuild index mapping when table is renamed
modified:
  storage/falcon/Configuration.cpp
  storage/falcon/StorageTable.cpp
  storage/falcon/StorageTableShare.cpp
  storage/falcon/StorageTableShare.h
  storage/falcon/ha_falcon.cpp
  storage/falcon/ha_falcon.h

 2813 Vladislav Vaintroub	2008-09-08
      Add missing dependency to perror  to avoid  compile error in multiprocessor builds
modified:
  extra/CMakeLists.txt

=== modified file 'storage/falcon/Configuration.cpp'
--- a/storage/falcon/Configuration.cpp	2008-08-29 20:41:13 +0000
+++ b/storage/falcon/Configuration.cpp	2008-09-08 19:46:25 +0000
@@ -42,7 +42,6 @@
 #include "SQLError.h"
 #include "Log.h"
 #include "IOx.h"
-#include "ScanDir.h"
 
 #ifndef ULL
 #define ULL(a)		((uint64) a)

=== modified file 'storage/falcon/StorageTable.cpp'
--- a/storage/falcon/StorageTable.cpp	2008-08-22 06:47:40 +0000
+++ b/storage/falcon/StorageTable.cpp	2008-09-08 19:46:25 +0000
@@ -195,7 +195,7 @@ int StorageTable::setCurrentIndex(int in
 		}
 	
 	if (!(currentIndex = share->getIndex(indexId)))
-{
+		{
 		clearCurrentIndex();
 		return StorageErrorNoIndex;
 		}

=== modified file 'storage/falcon/StorageTableShare.cpp'
--- a/storage/falcon/StorageTableShare.cpp	2008-09-03 22:17:54 +0000
+++ b/storage/falcon/StorageTableShare.cpp	2008-09-08 19:46:25 +0000
@@ -62,10 +62,10 @@ StorageIndexDesc::StorageIndexDesc()
 	rawName[0] = '\0';
 };
 
-StorageIndexDesc::StorageIndexDesc(const StorageIndexDesc *indexInfo)
+StorageIndexDesc::StorageIndexDesc(const StorageIndexDesc *indexDesc)
 {
-	if (indexInfo)
-		*this = *indexInfo;
+	if (indexDesc)
+		*this = *indexDesc;
 	else
 		{
 		id = 0;
@@ -86,6 +86,35 @@ StorageIndexDesc::~StorageIndexDesc(void
 {
 }
 
+bool StorageIndexDesc::operator==(const StorageIndexDesc &indexDesc) const
+{
+	if ( !(id			  == indexDesc.id
+		&& unique		  == indexDesc.unique
+		&& primaryKey 	  == indexDesc.primaryKey
+		&& numberSegments == indexDesc.numberSegments))
+		return false;
+
+	if (strcmp(indexDesc.name, name) != 0)
+		return false;
+	
+	/***
+	const char *q = indexDesc.name;
+	const char *p = name;
+	
+	for (; (*p && *q); ++p, ++q)
+		if (*p != *q)
+			return false;
+	***/
+
+	return true;
+}
+
+bool StorageIndexDesc::operator!=(const StorageIndexDesc &indexDesc) const
+{
+	return !(*this == indexDesc);
+}
+
+
 //////////////////////////////////////////////////////////////////////
 // Construction/Destruction
 //////////////////////////////////////////////////////////////////////
@@ -285,11 +314,17 @@ const char* StorageTableShare::cleanupTa
 	return buffer;
 }
 
-char* StorageTableShare::createIndexName(const char *rawName, char *indexName)
+char* StorageTableShare::createIndexName(const char *rawName, char *indexName, bool
primary)
 {
-	char nameBuffer[indexNameSize];
-	cleanupFieldName(rawName, nameBuffer, sizeof(nameBuffer));
-	sprintf(indexName, "%s$%s", name.getString(), nameBuffer);
+	if (primary)
+		strcpy(indexName, table->getPrimaryKeyName().getString());
+	else
+		{
+		char nameBuffer[indexNameSize];
+		cleanupFieldName(rawName, nameBuffer, sizeof(nameBuffer));
+		sprintf(indexName, "%s$%s", name.getString(), nameBuffer);
+		}
+	
 	return indexName;
 }
 
@@ -362,12 +397,35 @@ int StorageTableShare::dropIndex(Storage
 	
 	int ret = storageDatabase->dropIndex(storageConnection, table, sql);
 	
-	if (!ret)
-		deleteIndex(indexDesc->id);
+	deleteIndex(indexDesc->id);
 				
 	return ret;
 }
 
+bool StorageTableShare::validateIndex(int indexId, StorageIndexDesc *indexTarget)
+{
+	StorageIndexDesc *indexDesc = getIndex(indexId);
+	
+	if (!indexDesc || *indexDesc != *indexTarget)
+		return false;
+	
+	Index *index = table->findIndex(indexDesc->name);
+	
+	if (!index)
+		return false;
+	
+	if (index != indexDesc->index)
+		return false;
+	
+	if (indexTarget->primaryKey && index->type != PrimaryKey)
+		return false;
+			
+	if (index->recordsPerSegment != indexDesc->segmentRecordCounts)
+		return false;
+	
+	return true;
+}
+
 void StorageTableShare::deleteIndexes()
 {
 	for (StorageIndexDesc *indexDesc; (indexDesc = indexes);)
@@ -377,6 +435,16 @@ void StorageTableShare::deleteIndexes()
 		}
 }
 
+int StorageTableShare::numberIndexes()
+{
+	int indexCount = 0;
+	
+	for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next,
indexCount++)
+		;
+	
+	return indexCount;
+}
+
 int StorageTableShare::renameTable(StorageConnection *storageConnection, const char*
newName)
 {
 	char tableName[256];
@@ -402,24 +470,25 @@ int StorageTableShare::setIndex(const St
 
 	if (!getIndex(indexInfo->id))
 		{
-		StorageIndexDesc *indexDesc = new StorageIndexDesc(indexInfo);
-		addIndex(indexDesc);
 
-	// Find the corresponding Falcon index
-	
-	if (indexDesc->primaryKey)
-		indexDesc->index = table->primaryKey;
-	else
-		{
-		char indexName[indexNameSize];
-		sprintf(indexName, "%s$%s", name.getString(), indexDesc->name);
-		indexDesc->index = table->findIndex(indexName);
-		}
+		// Find the corresponding Falcon index else fail
 
-	if (indexDesc->index)
-		indexDesc->segmentRecordCounts = indexDesc->index->recordsPerSegment;
-	else
-		ret = StorageErrorNoIndex;
+		Index *index;
+		
+		if (indexInfo->primaryKey)
+			index = table->primaryKey;
+		else
+			index = table->findIndex(indexInfo->name);
+		
+		if (index)
+			{
+			StorageIndexDesc *indexDesc = new StorageIndexDesc(indexInfo);
+			indexDesc->index = index;
+			indexDesc->segmentRecordCounts = indexDesc->index->recordsPerSegment;
+			addIndex(indexDesc);
+			}
+		else
+			ret = StorageErrorNoIndex;
 		}
 		
 	return ret;
@@ -432,7 +501,10 @@ StorageIndexDesc* StorageTableShare::get
 			
 	for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next)
 		if (indexDesc->id == indexId)
+			{
+			ASSERT(indexDesc->index != NULL);
 			return indexDesc;
+			}
 		
 	return NULL;
 }
@@ -462,8 +534,11 @@ StorageIndexDesc* StorageTableShare::get
 	sync.lock(Shared);
 	
 	for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next)
-		if (indexDesc->name == name)
+		if (!strcmp(indexDesc->name, name))
+			{
+			ASSERT(indexDesc->index != NULL);
 			return indexDesc;
+			}
 			
 	return NULL;
 }

=== modified file 'storage/falcon/StorageTableShare.h'
--- a/storage/falcon/StorageTableShare.h	2008-08-22 06:47:40 +0000
+++ b/storage/falcon/StorageTableShare.h	2008-09-08 19:46:25 +0000
@@ -53,8 +53,10 @@ class StorageIndexDesc
 {
 public:
 	StorageIndexDesc();
-	StorageIndexDesc(const StorageIndexDesc *indexInfo);
+	StorageIndexDesc(const StorageIndexDesc *indexDesc);
 	virtual ~StorageIndexDesc(void);
+	bool operator==(const StorageIndexDesc &indexDesc) const;
+	bool operator!=(const StorageIndexDesc &indexDesc) const;
 	
 	int			id;
 	int			unique;
@@ -113,7 +115,9 @@ public:
 	virtual void		unlockIndexes(void);
 	virtual int			createIndex(StorageConnection *storageConnection, StorageIndexDesc
*indexDesc, const char *sql);
 	virtual int			dropIndex(StorageConnection *storageConnection, StorageIndexDesc
*indexDesc, const char *sql);
+	virtual bool		validateIndex(int indexId, StorageIndexDesc *indexTarget);
 	virtual void		deleteIndexes();
+	virtual int			numberIndexes();
 	virtual int			renameTable(StorageConnection *storageConnection, const char* newName);
 	virtual INT64		getSequenceValue(int delta);
 	virtual int			setSequenceValue(INT64 value);
@@ -148,7 +152,7 @@ public:
 
 	static const char*	getDefaultRoot(void);
 	static const char*	cleanupTableName(const char* name, char* buffer, int bufferLength,
char *schema, int schemaLength);
-	char*				createIndexName(const char *rawName, char *indexName);
+	char*				createIndexName(const char *rawName, char *indexName, bool primary = false);
 	
 	JString				name;
 	JString				schemaName;

=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp	2008-09-05 16:26:12 +0000
+++ b/storage/falcon/ha_falcon.cpp	2008-09-08 19:46:25 +0000
@@ -518,11 +518,11 @@ int StorageInterface::open(const char *n
 
 	thr_lock_data_init((THR_LOCK *)storageShare->impure, &lockData, NULL);
 
-	ret = setIndexes();
-	
 	if (table)
 		mapFields(table);
 	
+	setIndexes(table);
+	
 	if (ret)
 		DBUG_RETURN(error(ret));
 
@@ -850,6 +850,8 @@ int StorageInterface::create(const char 
 				}
 
 	mapFields(form);
+	
+	setIndexes(table);
 
 	DBUG_RETURN(0);
 }
@@ -868,12 +870,9 @@ int StorageInterface::createIndex(const 
 	StorageIndexDesc indexDesc;
 	getKeyDesc(table, indexId, &indexDesc);
 	
-	char indexName[indexNameSize];
-	storageShare->createIndexName(indexDesc.name, indexName);
-
 	CmdGen gen;
 	const char *unique = (key->flags & HA_NOSAME) ? "unique " : "";
-	gen.gen("create %sindex \"%s\" on %s.\"%s\" ", unique, indexName, schemaName,
tableName);
+	gen.gen("create %sindex \"%s\" on %s.\"%s\" ", unique, indexDesc.name, schemaName,
tableName);
 	genKeyFields(key, &gen);
 	const char *sql = gen.getString();
 
@@ -885,11 +884,8 @@ int StorageInterface::dropIndex(const ch
 	StorageIndexDesc indexDesc;
 	getKeyDesc(table, indexId, &indexDesc);
 	
-	char indexName[indexNameSize];
-	storageShare->createIndexName(indexDesc.name, indexName);
-
 	CmdGen gen;
-	gen.gen("drop index %s.\"%s\"", schemaName, indexName);
+	gen.gen("drop index %s.\"%s\"", schemaName, indexDesc.name);
 	const char *sql = gen.getString();
 
 	return storageTable->dropIndex(&indexDesc, sql);
@@ -1394,8 +1390,9 @@ int StorageInterface::index_init(uint id
 	nextRecord = 0;
 	haveStartKey = false;
 	haveEndKey = false;
+	int ret = 0;
 
-	int ret = storageTable->setCurrentIndex(idx);
+	ret = storageTable->setCurrentIndex(idx);
 
 	if (ret)
 		{
@@ -1403,6 +1400,8 @@ int StorageInterface::index_init(uint id
 		ret = storageTable->setCurrentIndex(idx);
 		}
 		
+	// validateIndexes(table);
+		
 	if (ret)
 		DBUG_RETURN(error(ret));
 
@@ -1465,22 +1464,18 @@ void StorageInterface::getKeyDesc(TABLE 
 {
 	KEY *keyInfo = table->key_info + indexId;
 	int numberKeys = keyInfo->key_parts;
-	char nameBuffer[indexNameSize];
+	
+	indexDesc->id			  = indexId;
+	indexDesc->numberSegments = numberKeys;
+	indexDesc->unique		  = (keyInfo->flags & HA_NOSAME);
+	indexDesc->primaryKey	  = (table->s->primary_key == (uint)indexId);
 	
 	// Clean up the index name for internal use
 	
 	strncpy(indexDesc->rawName, (const char*)keyInfo->name, MIN(indexNameSize,
(int)strlen(keyInfo->name)+1));
-	storageShare->cleanupFieldName(indexDesc->rawName, nameBuffer,
sizeof(nameBuffer));
 	indexDesc->rawName[indexNameSize-1] = '\0';
+	storageShare->createIndexName(indexDesc->rawName, indexDesc->name,
indexDesc->primaryKey);
 	
-	strncpy(indexDesc->name, (const char*)nameBuffer, MIN(indexNameSize,
(int)strlen(nameBuffer)+1));
-	indexDesc->name[indexNameSize-1] = '\0';
-
-	indexDesc->id			  = indexId;
-	indexDesc->numberSegments = numberKeys;
-	indexDesc->unique		  = (keyInfo->flags & HA_NOSAME);
-	indexDesc->primaryKey	  = (table->s->primary_key == (uint)indexId);
-
 	for (int n = 0; n < numberKeys; ++n)
 		{
 		StorageSegment *segment = indexDesc->segments + n;
@@ -1519,8 +1514,17 @@ int StorageInterface::rename_table(const
 	if (ret)
 		DBUG_RETURN(error(ret));
 
+	storageTable->clearCurrentIndex();
+	storageShare->lockIndexes(true);
+	storageShare->lock(true);
+
 	ret = storageShare->renameTable(storageConnection, to);
 	
+	remapIndexes(table);
+	
+	storageShare->unlock();
+	storageShare->unlockIndexes();
+
 	if (ret)
 		DBUG_RETURN(error(ret));
 
@@ -1541,6 +1545,7 @@ int StorageInterface::read_range_first(c
 	storageTable->clearIndexBounds();
 	haveStartKey = false;
 	haveEndKey = false;
+	int ret = 0;
 
 	if (start_key && !storageTable->isKeyNull((const unsigned char*)
start_key->key, start_key->length))
 		{
@@ -1563,7 +1568,7 @@ int StorageInterface::read_range_first(c
 		if (end_key->flag == HA_READ_BEFORE_KEY)
 			storageTable->setReadBeforeKey();
 
-		int ret = storageTable->setIndexBound((const unsigned char*) end_key->key,
+		ret = storageTable->setIndexBound((const unsigned char*) end_key->key,
 												end_key->length, UpperBound);
 		if (ret)
 			DBUG_RETURN(error(ret));
@@ -2109,10 +2114,9 @@ int StorageInterface::check_if_supported
 	DBUG_ENTER("StorageInterface::check_if_supported_alter");
 	tempTable = (create_info->options & HA_LEX_CREATE_TMP_TABLE) ? true : false;
 	HA_ALTER_FLAGS supported;
-	supported = supported | HA_ADD_INDEX | HA_DROP_INDEX;
+	supported = supported | HA_ADD_INDEX | HA_DROP_INDEX | HA_ADD_UNIQUE_INDEX |
HA_DROP_UNIQUE_INDEX;
 						/**
-						| HA_ADD_COLUMN | HA_ADD_UNIQUE_INDEX | HA_DROP_UNIQUE_INDEX
-						| HA_COLUMN_STORAGE | HA_COLUMN_FORMAT;
+						| HA_ADD_COLUMN | HA_COLUMN_STORAGE | HA_COLUMN_FORMAT;
 						**/
 	HA_ALTER_FLAGS notSupported = ~(supported);
 	
@@ -2153,7 +2157,7 @@ int StorageInterface::check_if_supported
 	// 1. Check for supported ALTER combinations
 	// 2. Can error message be improved for non-null columns?
 	
-	if (alter_flags->is_set(HA_ADD_INDEX))
+	if (alter_flags->is_set(HA_ADD_INDEX) || alter_flags->is_set(HA_ADD_UNIQUE_INDEX))
 		{
 		for (unsigned int n = 0; n < altered_table->s->keys; n++)
 			{
@@ -2185,7 +2189,7 @@ int StorageInterface::check_if_supported
 			}
 		}
 		
-	if (alter_flags->is_set(HA_DROP_INDEX))
+	if (alter_flags->is_set(HA_DROP_INDEX) ||
alter_flags->is_set(HA_DROP_UNIQUE_INDEX))
 		{
 		}
 		
@@ -2200,10 +2204,10 @@ int StorageInterface::alter_table_phase1
 	if (alter_flags->is_set(HA_ADD_COLUMN))
 		ret = addColumn(thd, altered_table, create_info, alter_info, alter_flags);
 		
-	if (alter_flags->is_set(HA_ADD_INDEX) && !ret)
+	if ((alter_flags->is_set(HA_ADD_INDEX) ||
alter_flags->is_set(HA_ADD_UNIQUE_INDEX)) && !ret)
 		ret = addIndex(thd, altered_table, create_info, alter_info, alter_flags);
 		
-	if (alter_flags->is_set(HA_DROP_INDEX) && !ret)
+	if ((alter_flags->is_set(HA_DROP_INDEX) ||
alter_flags->is_set(HA_DROP_UNIQUE_INDEX)) && !ret)
 		ret = dropIndex(thd, altered_table, create_info, alter_info, alter_flags);
 		
 	DBUG_RETURN(ret);
@@ -2386,7 +2390,7 @@ int StorageInterface::setIndex(TABLE *ta
 	return storageTable->setIndex(&indexDesc);
 }
 
-int StorageInterface::setIndexes(void)
+int StorageInterface::setIndexes(TABLE *table)
 {
 	int ret = 0;
 	
@@ -2394,14 +2398,14 @@ int StorageInterface::setIndexes(void)
 		return ret;
 
 	storageShare->lockIndexes(true);
+	storageShare->lock(true);
 
-	if (!storageShare->haveIndexes(table->s->keys))
-		for (uint n = 0; n < table->s->keys; ++n)
-			if ((ret = setIndex(table, n)))
-				break;
+	ret = remapIndexes(table);
+	// validateIndexes(table, true);
 
+	storageShare->unlock();
 	storageShare->unlockIndexes();
-
+	
 	return ret;
 }
 
@@ -2409,11 +2413,11 @@ int StorageInterface::remapIndexes(TABLE
 {
 	int ret = 0;
 	
+	storageShare->deleteIndexes();
+
 	if (!table)
 		return ret;
 		
-	storageShare->deleteIndexes();
-
 	for (uint n = 0; n < table->s->keys; ++n)
 		if ((ret = setIndex(table, n)))
 			break;
@@ -2421,6 +2425,32 @@ int StorageInterface::remapIndexes(TABLE
 	return ret;
 }
 
+bool StorageInterface::validateIndexes(TABLE *table, bool exclusiveLock)
+{
+	bool ret = true;
+	
+	if (!table)
+		return false;
+	
+	storageShare->lockIndexes(exclusiveLock);
+		
+	for (uint n = 0; (n < table->s->keys) && ret; ++n)
+		{
+		StorageIndexDesc indexDesc;
+		getKeyDesc(table, n, &indexDesc);
+		
+		if (!storageShare->validateIndex(n, &indexDesc))
+			ret = false;
+		}
+	
+	if (ret && (table->s->keys != (uint)storageShare->numberIndexes()))
+		ret = false;
+	
+	storageShare->unlockIndexes();
+
+	return ret;
+}
+
 int StorageInterface::genTable(TABLE* table, CmdGen* gen)
 {
 	const char *tableName = storageTable->getName();

=== modified file 'storage/falcon/ha_falcon.h'
--- a/storage/falcon/ha_falcon.h	2008-09-03 22:17:54 +0000
+++ b/storage/falcon/ha_falcon.h	2008-09-08 19:46:25 +0000
@@ -122,8 +122,9 @@ public:
 	int				error(int storageError);
 	void			freeActiveBlobs(void);
 	int				setIndex(TABLE *table, int indexId);
-	int				setIndexes(void);
+	int				setIndexes(TABLE *table);
 	int				remapIndexes(TABLE *table);
+	bool			validateIndexes(TABLE *table, bool exclusiveLock = false);
 	int				genTable(TABLE* table, CmdGen* gen);
 	int				genType(Field *field, CmdGen *gen);
 	void			genKeyFields(KEY *key, CmdGen *gen);