From: Date: August 22 2008 12:52am Subject: bzr commit into mysql-6.0-falcon branch (cpowers:2793) Bug#38041 List-Archive: http://lists.mysql.com/commits/52263 X-Bug: 38041 Message-Id: <20080821225221.D97291DB0729@xeno.mysql.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///home/cpowers/work/dev/dev-05/mysql/ 2793 Christopher Powers 2008-08-21 Bug#38041 "Bizarre errors when ALTER ADD/DROP KEY on Falcon tables" Resolved memory corruption stemming from incompatible allocation between Falcon and the StorageInterface. - Converted JString fields in StorageIndexDesc to char[] - Changed StorageTableShare::indexes[] from DenseArray to linked list - Reverted DenseArray to original removed: storage/falcon/Engine.vcproj modified: storage/falcon/DenseArray.h storage/falcon/StorageTable.cpp storage/falcon/StorageTable.h storage/falcon/StorageTableShare.cpp storage/falcon/StorageTableShare.h storage/falcon/ha_falcon.cpp storage/falcon/ha_falcon.h per-file messages: storage/falcon/DenseArray.h Reverted storage/falcon/StorageTable.cpp Changes associated with making StorageTableShare::indexes[] a linked list storage/falcon/StorageTable.h Changes associated with making StorageTableShare::indexes[] a linked list storage/falcon/StorageTableShare.cpp Converted StorageTableShare::indexes[] from DenseArray to linked list Added associated methods for managing ::indexes, add/delete, etc. Added StorageIndexDesc constructors storage/falcon/StorageTableShare.h nverted StorageTableShare::indexes[] from DenseArray to linked list Added associated methods for managing ::indexes, add/delete, etc. Changed name fields in StorageIndexDesc from JString to char[] storage/falcon/ha_falcon.cpp Remap MySQL/Falcon indexes after each create/drop index Externally lock StorageTableShare in drop/createIndex() Reverted StorageIndexDesc name fields from JString to char[] to avoid mismatched memory allocation/deallocation between StorageInterface and Falcon. storage/falcon/ha_falcon.h Added remapIndexes() === modified file 'storage/falcon/DenseArray.h' --- a/storage/falcon/DenseArray.h 2008-08-18 05:45:29 +0000 +++ b/storage/falcon/DenseArray.h 2008-08-21 22:52:10 +0000 @@ -47,35 +47,17 @@ public: if (newLength < length) return; - T *newVector = new T[newLength]; - T *oldVector = vector; - memcpy((void*) newVector, (void*) vector, length * sizeof(T)); - memset((void*) (newVector + length), 0, (newLength - length) * sizeof(T)); - vector = newVector; - int oldLength = length; - length = newLength; - memset((void*) oldVector, 0xbc, oldLength * sizeof(T)); - delete [] oldVector; - - /** T *oldVector = vector; vector = new T[newLength]; memcpy((void*) vector, (void*) oldVector, length * sizeof(T)); memset((void*) (vector + length), 0, (newLength - length) * sizeof(T)); length = newLength; - **/ }; void zap () { memset((void*) vector, 0, length * sizeof(T)); }; - - void zap (uint n) - { - if (n < length) - memset(vector + n, 0, sizeof(T)); - } T get (uint index) { === removed file 'storage/falcon/Engine.vcproj' --- a/storage/falcon/Engine.vcproj 2007-12-04 15:38:41 +0000 +++ b/storage/falcon/Engine.vcproj 1970-01-01 00:00:00 +0000 @@ -1,9227 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - === modified file 'storage/falcon/StorageTable.cpp' --- a/storage/falcon/StorageTable.cpp 2008-08-19 03:33:01 +0000 +++ b/storage/falcon/StorageTable.cpp 2008-08-21 22:52:10 +0000 @@ -139,9 +139,9 @@ int StorageTable::updateRow(int recordNu return 0; } -int StorageTable::createIndex(StorageIndexDesc *indexDesc, int indexCount, const char *sql) +int StorageTable::createIndex(StorageIndexDesc *indexDesc, const char *sql) { - return share->createIndex(storageConnection, indexDesc, indexCount, sql); + return share->createIndex(storageConnection, indexDesc, sql); } int StorageTable::dropIndex(StorageIndexDesc *indexDesc, const char *sql) @@ -193,7 +193,7 @@ int StorageTable::setCurrentIndex(int in } if (!(currentIndex = share->getIndex(indexId))) -{ + { clearCurrentIndex(); return StorageErrorNoIndex; } @@ -217,9 +217,9 @@ int StorageTable::clearCurrentIndex() return 0; } -int StorageTable::setIndex(int indexCount, StorageIndexDesc* indexDesc) +int StorageTable::setIndex(StorageIndexDesc* indexDesc) { - return share->setIndex(indexCount, indexDesc); + return share->setIndex(indexDesc); } int StorageTable::indexScan(int indexOrder) === modified file 'storage/falcon/StorageTable.h' --- a/storage/falcon/StorageTable.h 2008-08-18 05:45:29 +0000 +++ b/storage/falcon/StorageTable.h 2008-08-21 22:52:10 +0000 @@ -78,7 +78,7 @@ public: virtual int indexScan(int indexOrder); virtual int setCurrentIndex(int indexId); virtual int clearCurrentIndex(); - virtual int setIndex(int indexCount, StorageIndexDesc* indexDesc); + virtual int setIndex(StorageIndexDesc* indexDesc); virtual void indexEnd(void); virtual int setIndexBound(const unsigned char* key, int keyLength, int which); virtual int storeBlob(StorageBlob* blob); @@ -94,7 +94,7 @@ public: virtual int fetch(int recordNumber, bool lockForUpdate); virtual int updateRow(int recordNumber); - virtual int createIndex(StorageIndexDesc *indexDesc, int indexCount, const char *sql); + virtual int createIndex(StorageIndexDesc *indexDesc, const char *sql); virtual int dropIndex(StorageIndexDesc *indexDesc, const char *sql); virtual const unsigned char* getEncoding(int fieldIndex); virtual const char* getName(void); === modified file 'storage/falcon/StorageTableShare.cpp' --- a/storage/falcon/StorageTableShare.cpp 2008-08-20 19:40:10 +0000 +++ b/storage/falcon/StorageTableShare.cpp 2008-08-21 22:52:10 +0000 @@ -49,6 +49,43 @@ static const char *DB_ROOT = ".fts"; static const char THIS_FILE[]=__FILE__; #endif +StorageIndexDesc::StorageIndexDesc() +{ + id = 0; + unique = 0; + primaryKey = 0; + numberSegments = 0; + index = NULL; + segmentRecordCounts = NULL; + next = NULL; + name[0] = '\0'; + rawName[0] = '\0'; +}; + +StorageIndexDesc::StorageIndexDesc(const StorageIndexDesc *indexInfo) +{ + if (indexInfo) + *this = *indexInfo; + else + { + id = 0; + unique = 0; + primaryKey = 0; + numberSegments = 0; + segmentRecordCounts = NULL; + name[0] = '\0'; + rawName[0] = '\0'; + } + + index = NULL; + next = NULL; + prev = NULL; +}; + +StorageIndexDesc::~StorageIndexDesc(void) +{ +} + ////////////////////////////////////////////////////////////////////// // Construction/Destruction ////////////////////////////////////////////////////////////////////// @@ -68,7 +105,7 @@ StorageTableShare::StorageTableShare(Sto sequence = NULL; tempTable = tempTbl; setPath(path); - numberIndexes = 0; + indexes = NULL; if (tempTable) tableSpace = TEMPORARY_TABLESPACE; @@ -87,9 +124,11 @@ StorageTableShare::~StorageTableShare(vo if (storageDatabase) storageDatabase->release(); - for (uint n = 0; n < indexes.length; n++) - if (indexes.vector[n]) - delete indexes.get(n); + for (StorageIndexDesc *indexDesc; (indexDesc = indexes);) + { + indexes = indexDesc->next; + delete indexDesc; + } } void StorageTableShare::lock(bool exclusiveLock) @@ -254,12 +293,12 @@ char* StorageTableShare::createIndexName return indexName; } -int StorageTableShare::createIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, int indexCount, const char *sql) +int StorageTableShare::createIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, const char *sql) { if (!table) open(); - // Always get syncIndexes before syncObject + // Lock out other clients before locking the table Sync syncIndex(syncIndexes, "StorageTableShare::createIndex(1)"); syncIndex.lock(Exclusive); @@ -270,18 +309,51 @@ int StorageTableShare::createIndex(Stora int ret = storageDatabase->createIndex(storageConnection, table, sql); if (!ret) - ret = setIndex(indexCount, indexDesc); + ret = setIndex(indexDesc); return ret; } +void StorageTableShare::addIndex(StorageIndexDesc *indexDesc) +{ + if (!getIndex(indexDesc->id)) + { + if (indexes) + { + indexDesc->next = indexes; + indexDesc->prev = NULL; + indexes->prev = indexDesc; + } + + indexes = indexDesc; + } +} + +void StorageTableShare::deleteIndex(int indexId) +{ + for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next) + if (indexDesc->id == indexId) + { + if (indexDesc->prev) + indexDesc->prev->next = indexDesc->next; + else + indexes = indexDesc->next; + + if (indexDesc->next) + indexDesc->next->prev = indexDesc->prev; + + delete indexDesc; + break; + } +} + int StorageTableShare::dropIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, const char *sql) { if (!table) open(); - // Always get syncIndexes before syncObject - + // Lock out other clients before locking the table + Sync syncIndex(syncIndexes, "StorageTableShare::dropIndex(1)"); syncIndex.lock(Exclusive); @@ -291,11 +363,20 @@ int StorageTableShare::dropIndex(Storage int ret = storageDatabase->dropIndex(storageConnection, table, sql); if (!ret) - clearIndex(indexDesc); + deleteIndex(indexDesc->id); return ret; } +void StorageTableShare::deleteIndexes() +{ + for (StorageIndexDesc *indexDesc; (indexDesc = indexes);) + { + indexes = indexDesc->next; + delete indexDesc; + } +} + int StorageTableShare::renameTable(StorageConnection *storageConnection, const char* newName) { char tableName[256]; @@ -315,130 +396,109 @@ int StorageTableShare::renameTable(Stora return ret; } -void StorageTableShare::resizeIndexes(int indexCount) +int StorageTableShare::setIndex(const StorageIndexDesc *indexInfo) { - if (indexCount <= 0) - return; + int ret = 0; - if ((uint)indexCount > indexes.length) - indexes.extend(indexCount + 5); - - numberIndexes = indexCount; -} - -int StorageTableShare::setIndex(int indexCount, const StorageIndexDesc *indexInfo) + if (!getIndex(indexInfo->id)) { - int indexId = indexInfo->id; + StorageIndexDesc *indexDesc = new StorageIndexDesc(indexInfo); + addIndex(indexDesc); - if ((uint)indexId >= indexes.length || numberIndexes < indexCount) - resizeIndexes(indexCount); - - // Allocate a new index if necessary - - StorageIndexDesc *indexDesc = indexes.get(indexId); - - if (!indexDesc) - indexes.vector[indexId] = indexDesc = new StorageIndexDesc(indexId); - - // Copy index description info + // Find the corresponding Falcon index - *indexDesc = *indexInfo; - - // Find the corresponding Falcon index - - if (indexDesc->primaryKey) - indexDesc->index = table->primaryKey; - else - { - char indexName[indexNameSize]; - sprintf(indexName, "%s$%s", name.getString(), indexDesc->name.getString()); - indexDesc->index = table->findIndex(indexName); - } - - int ret = 0; - - if (indexDesc->index) - indexDesc->segmentRecordCounts = indexDesc->index->recordsPerSegment; - else - ret = StorageErrorNoIndex; - - ASSERT((!ret ? validateIndexes() : true)); - - return ret; -} - -void StorageTableShare::clearIndex(StorageIndexDesc *indexDesc) -{ - if (numberIndexes > 0) - { - for (int n = indexDesc->id; n < numberIndexes-1; n++) + if (indexDesc->primaryKey) + indexDesc->index = table->primaryKey; + else { - indexes.vector[n] = indexes.vector[n+1]; - indexes.vector[n]->id = n; // assume that index id will match server + char indexName[indexNameSize]; + sprintf(indexName, "%s$%s", name.getString(), indexDesc->name); + indexDesc->index = table->findIndex(indexName); } - - indexes.zap(numberIndexes-1); - numberIndexes--; - } - - ASSERT(validateIndexes()); -} -bool StorageTableShare::validateIndexes() -{ - for (int n = 0; n < numberIndexes; n++) - { - StorageIndexDesc *indexDesc = indexes.get(n); - if (indexDesc && indexDesc->id != n) - return false; + if (indexDesc->index) + indexDesc->segmentRecordCounts = indexDesc->index->recordsPerSegment; + else + ret = StorageErrorNoIndex; } - - return true; + + return ret; } -// Assumes syncIndexes is locked - StorageIndexDesc* StorageTableShare::getIndex(int indexId) { - if (!indexes.length || indexId >= numberIndexes) + if (!indexes) return NULL; - - return indexes.get(indexId); + + for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next) + if (indexDesc->id == indexId) + return indexDesc; + + return NULL; } StorageIndexDesc* StorageTableShare::getIndex(int indexId, StorageIndexDesc *indexDesc) { - StorageIndexDesc *index; + if (!indexes) + return NULL; - if (!indexes.length || indexId >= numberIndexes) - index = NULL; - else - { - Sync sync(syncIndexes, "StorageTableShare::getIndex"); - sync.lock(Shared); + Sync sync(syncIndexes, "StorageTableShare::getIndex"); + sync.lock(Shared); - index = indexes.get(indexId); + StorageIndexDesc *index = getIndex(indexId); - if (index) - *indexDesc = *index; - } + if (index) + *indexDesc = *index; return index; } StorageIndexDesc* StorageTableShare::getIndex(const char *name) { + if (!indexes) + return NULL; + Sync sync(syncIndexes, "StorageTableShare::getIndex(name)"); sync.lock(Shared); - for (int i = 0; i < numberIndexes; i++) - { - StorageIndexDesc *indexDesc = indexes.get(i); - if (indexDesc && indexDesc->name == name) + for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next) + if (indexDesc->name == name) return indexDesc; + + return NULL; +} + +int StorageTableShare::getIndexId(const char* schemaName, const char* indexName) +{ + if (!indexes) + return -1; + + for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next) + { + Index *index = indexDesc->index; + + if (index) + if (strcmp(index->getIndexName(), indexName) == 0 && + strcmp(index->getSchemaName(), schemaName) == 0) + return indexDesc->id; } + + return -1; +} - return NULL; +int StorageTableShare::haveIndexes(int indexCount) +{ + if (!indexes) + return false; + + int n = 0; + for (StorageIndexDesc *indexDesc = indexes; indexDesc; indexDesc = indexDesc->next, n++) + { + if (!indexDesc->index) + return false; + } + + return (n == indexCount); } INT64 StorageTableShare::getSequenceValue(int delta) @@ -464,45 +524,6 @@ int StorageTableShare::setSequenceValue( return 0; } -// Get index id using the internal (Falcon) index name - -int StorageTableShare::getIndexId(const char* schemaName, const char* indexName) -{ - if (indexes.length > 0) - for (int n = 0; n < numberIndexes; ++n) - { - Index *index = indexes.get(n)->index; - - if (strcmp(index->getIndexName(), indexName) == 0 && - strcmp(index->getSchemaName(), schemaName) == 0) - return n; - } - - return -1; -} - -int StorageTableShare::haveIndexes(int indexCount) -{ - if (indexes.length == 0) - return false; - - if (indexCount > numberIndexes) - return false; - - for (int n = 0; n < numberIndexes; ++n) - { - StorageIndexDesc* index = indexes.get(n); - - if (!index) - return false; - - if (index && !index->index) - return false; - } - - return true; -} - void StorageTableShare::setTablePath(const char* path, bool tmp) { if (pathName.IsEmpty()) @@ -713,4 +734,4 @@ int StorageTableShare::getFieldId(const return -1; return field->id; -} +} \ No newline at end of file === modified file 'storage/falcon/StorageTableShare.h' --- a/storage/falcon/StorageTableShare.h 2008-08-18 05:45:29 +0000 +++ b/storage/falcon/StorageTableShare.h 2008-08-21 22:52:10 +0000 @@ -18,7 +18,6 @@ #include "JString.h" #include "SyncObject.h" -#include "DenseArray.h" #ifndef _WIN32 #define __int64 long long @@ -49,23 +48,28 @@ struct StorageSegment { void *mysql_charset; }; +// StorageIndexDesc maps a server-side index to a Falcon index + class StorageIndexDesc { public: - StorageIndexDesc(int indexId=0) : id (indexId), unique(0), primaryKey(0), numberSegments(0), /*name(NULL),*/ index(NULL), segmentRecordCounts(NULL){}; + StorageIndexDesc(); + StorageIndexDesc(const StorageIndexDesc *indexInfo); + virtual ~StorageIndexDesc(void); - int id;//cwp + int id; int unique; int primaryKey; int numberSegments; - JString name; // clean name - JString rawName; // original name + char name[indexNameSize]; // clean name + char rawName[indexNameSize]; // original name Index *index; uint64 *segmentRecordCounts; StorageSegment segments[MaxIndexSegments]; + StorageIndexDesc *next; + StorageIndexDesc *prev; }; - enum StorageError { StorageErrorRecordNotFound = -1, StorageErrorDupKey = -2, @@ -107,8 +111,9 @@ public: virtual void unlock(void); virtual void lockIndexes(bool exclusiveLock=false); virtual void unlockIndexes(void); - virtual int createIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, int indexCount, const char *sql); + virtual int createIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, const char *sql); virtual int dropIndex(StorageConnection *storageConnection, StorageIndexDesc *indexDesc, const char *sql); + virtual void deleteIndexes(); virtual int renameTable(StorageConnection *storageConnection, const char* newName); virtual INT64 getSequenceValue(int delta); virtual int setSequenceValue(INT64 value); @@ -118,10 +123,10 @@ public: virtual void registerCollation(const char* collationName, void* arg); int open(void); - void resizeIndexes(int indexCount); - int setIndex(int indexCount, const StorageIndexDesc* indexInfo); + void addIndex(StorageIndexDesc *indexDesc); + void deleteIndex(int indexId); + int setIndex(const StorageIndexDesc* indexInfo); void clearIndex(StorageIndexDesc *indexDesc); - bool validateIndexes(); StorageIndexDesc* getIndex(int indexId); StorageIndexDesc* getIndex(int indexId, StorageIndexDesc *indexDesc); StorageIndexDesc* getIndex(const char *name); @@ -159,10 +164,9 @@ public: StorageDatabase *storageDatabase; StorageHandler *storageHandler; Table *table; - DenseArray indexes; + StorageIndexDesc *indexes; Sequence *sequence; Format *format; // format for insertion - int numberIndexes; bool tempTable; int getFieldId(const char* fieldName); }; === modified file 'storage/falcon/ha_falcon.cpp' --- a/storage/falcon/ha_falcon.cpp 2008-08-19 14:27:42 +0000 +++ b/storage/falcon/ha_falcon.cpp 2008-08-21 22:52:10 +0000 @@ -670,7 +670,6 @@ int StorageInterface::info(uint what) DBUG_RETURN(0); } - void StorageInterface::getDemographics(void) { DBUG_ENTER("StorageInterface::getDemographics"); @@ -869,11 +868,11 @@ int StorageInterface::add_index(TABLE* t int StorageInterface::createIndex(const char *schemaName, const char *tableName, TABLE *table, int indexId) { KEY *key = table->key_info + indexId; - StorageIndexDesc indexDesc(indexId); + StorageIndexDesc indexDesc; getKeyDesc(table, indexId, &indexDesc); char indexName[indexNameSize]; - storageShare->createIndexName(indexDesc.name.getString(), indexName); + storageShare->createIndexName(indexDesc.name, indexName); CmdGen gen; const char *unique = (key->flags & HA_NOSAME) ? "unique " : ""; @@ -881,16 +880,16 @@ int StorageInterface::createIndex(const genKeyFields(key, &gen); const char *sql = gen.getString(); - return storageTable->createIndex(&indexDesc, table->s->keys, sql); + return storageTable->createIndex(&indexDesc, sql); } int StorageInterface::dropIndex(const char *schemaName, const char *tableName, TABLE *table, int indexId) { - StorageIndexDesc indexDesc(indexId); + StorageIndexDesc indexDesc; getKeyDesc(table, indexId, &indexDesc); char indexName[indexNameSize]; - storageShare->createIndexName(indexDesc.name.getString(), indexName); + storageShare->createIndexName(indexDesc.name, indexName); CmdGen gen; gen.gen("drop index %s.\"%s\"", schemaName, indexName); @@ -987,7 +986,10 @@ int StorageInterface::delete_table(const if (storageShare) { -// storageShare->lockIndexes(true); + + // Lock out other clients before locking the table + + storageShare->lockIndexes(true); storageShare->lock(true); if (storageShare->initialized) @@ -998,7 +1000,7 @@ int StorageInterface::delete_table(const } storageShare->unlock(); -// storageShare->unlockIndexes(); + storageShare->unlockIndexes(); } int res = storageTable->deleteTable(); @@ -1395,6 +1397,12 @@ int StorageInterface::index_init(uint id int ret = storageTable->setCurrentIndex(idx); if (ret) + { + setIndex(table, idx); + ret = storageTable->setCurrentIndex(idx); + } + + if (ret) DBUG_RETURN(error(ret)); DBUG_RETURN(ret); @@ -1458,9 +1466,16 @@ void StorageInterface::getKeyDesc(TABLE int numberKeys = keyInfo->key_parts; char nameBuffer[indexNameSize]; - indexDesc->rawName = keyInfo->name; + // Clean up the index name for internal use + + strncpy(indexDesc->rawName, (const char*)keyInfo->name, MIN(indexNameSize, strlen(keyInfo->name)+1)); storageShare->cleanupFieldName(indexDesc->rawName, nameBuffer, sizeof(nameBuffer)); - indexDesc->name = nameBuffer; + indexDesc->rawName[indexNameSize-1] = '\0'; + + strncpy(indexDesc->name, (const char*)nameBuffer, MIN(indexNameSize, 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); @@ -2232,6 +2247,11 @@ int StorageInterface::addIndex(THD* thd, const char *tableName = storageTable->getName(); const char *schemaName = storageTable->getSchemaName(); + // Lock out other clients before locking the table + + storageShare->lockIndexes(true); + storageShare->lock(true); + // Find indexes to be added by comparing table and alteredTable for (unsigned int n = 0; n < alteredTable->s->keys; n++) @@ -2248,11 +2268,19 @@ int StorageInterface::addIndex(THD* thd, if (tableKey >= tableEnd) if ((ret = createIndex(schemaName, tableName, alteredTable, n))) - return (error(ret)); + break; } } - return 0; + // The server indexes may have been reordered, so remap to the Falcon indexes + + if (!ret) + remapIndexes(alteredTable); + + storageShare->unlock(); + storageShare->unlockIndexes(); + + return error(ret); } int StorageInterface::dropIndex(THD* thd, TABLE* alteredTable, HA_CREATE_INFO* createInfo, HA_ALTER_INFO* alterInfo, HA_ALTER_FLAGS* alterFlags) @@ -2261,6 +2289,11 @@ int StorageInterface::dropIndex(THD* thd const char *tableName = storageTable->getName(); const char *schemaName = storageTable->getSchemaName(); + // Lock out other clients before locking the table + + storageShare->lockIndexes(true); + storageShare->lock(true); + // Find indexes to be dropped by comparing table and alteredTable for (unsigned int n = 0; n < table->s->keys; n++) @@ -2277,11 +2310,19 @@ int StorageInterface::dropIndex(THD* thd if (alterKey >= alterEnd) if ((ret = dropIndex(schemaName, tableName, table, n))) - return (error(ret)); + break; } } - return 0; + // The server indexes have been reordered, so remap to the Falcon indexes + + if (!ret) + remapIndexes(alteredTable); + + storageShare->unlock(); + storageShare->unlockIndexes(); + + return error(ret); } uint StorageInterface::max_supported_key_length(void) const @@ -2319,10 +2360,10 @@ void StorageInterface::logger(int mask, int StorageInterface::setIndex(TABLE *table, int indexId) { - StorageIndexDesc indexDesc(indexId); + StorageIndexDesc indexDesc; getKeyDesc(table, indexId, &indexDesc); - return storageTable->setIndex(table->s->keys, &indexDesc); + return storageTable->setIndex(&indexDesc); } int StorageInterface::setIndexes(void) @@ -2344,6 +2385,22 @@ int StorageInterface::setIndexes(void) return ret; } +int StorageInterface::remapIndexes(TABLE *table) +{ + int ret = 0; + + if (!table) + return ret; + + storageShare->deleteIndexes(); + + for (uint n = 0; n < table->s->keys; ++n) + if ((ret = setIndex(table, n))) + break; + + 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-08-18 05:45:29 +0000 +++ b/storage/falcon/ha_falcon.h 2008-08-21 22:52:10 +0000 @@ -124,6 +124,7 @@ public: void freeActiveBlobs(void); int setIndex(TABLE *table, int indexId); int setIndexes(void); + int remapIndexes(TABLE *table); int genTable(TABLE* table, CmdGen* gen); int genType(Field *field, CmdGen *gen); void genKeyFields(KEY *key, CmdGen *gen);