Chris,
It looks like you activated debug messages for deleteIndexEntry() in
Index::garbageCollect() Do you want to push that to the team tree?
You changed LOAD_AUTOCOMMIT_RECORDS from 10,000 to 100,000. Do you have
some new sense of harmonic balance and enlightenment or is this just the
best thing for xeno?
Christopher Powers wrote:
> #At file:///home/cpowers/work/dev/dev-02/mysql/
>
> 2957 Christopher Powers 2009-01-14
> Bug #42099 Falcon: Online ALTER add/drop primary key
>
> - Implemented online alter add/drop primary key
> - Improved index mapping between server and Falon
> -
> modified:
> mysql-test/suite/falcon/r/falcon_online_index.result
> mysql-test/suite/falcon/t/falcon_online_index.test
> storage/falcon/Database.cpp
> storage/falcon/Index.cpp
> storage/falcon/Statement.cpp
> storage/falcon/StorageTable.cpp
> storage/falcon/StorageTable.h
> storage/falcon/StorageTableShare.cpp
> storage/falcon/Table.cpp
> storage/falcon/Table.h
> storage/falcon/ha_falcon.cpp
> storage/falcon/ha_falcon.h
>
> per-file messages:
> mysql-test/suite/falcon/r/falcon_online_index.result
> Allow online add/drop primary key
> mysql-test/suite/falcon/t/falcon_online_index.test
> Allow online add/drop primary key
> storage/falcon/Database.cpp
> Databae::openDatabase() - Replaced FOR_INDEXES with FOR_ALL_INDEXES
> storage/falcon/Index.cpp
> Index::garbageCollect()
>
> Added comments, disabled redundant code.
> storage/falcon/Statement.cpp
> Changes for online add/drop primary key.
> Statement::upgradeTable() now commits and populates new indexes in
> separate system transactions.
> storage/falcon/StorageTable.cpp
> Added StorageTable::checkCurrentIndex() to verify that an index has been
> initialized for the current thread.
>
> Also replaced StorageErrorNoIndex return codes with more benign error,
> StorageErrorRecordNotFound until deficiencies in server's handling of online
> DDL are resolved. Bug TBD.
> storage/falcon/StorageTable.h
> Added StorageTable::checkCurrentIndex()
> storage/falcon/StorageTableShare.cpp
> StorageTableShare::dropIndex() removes index mapping regardless of error
> from StorageDatabase::dropIndex()
> storage/falcon/Table.cpp
> Replaced FOR_INDEXES with FOR_ALL_INDEXES in create()
> storage/falcon/Table.h
> Modified FOR_INDEXES macro to ignore indexId == -1, which indicates an incomplete
> index
> Added FOR_ALL_INDEXES macro for use when indexId == -1 is acceptable
> storage/falcon/ha_falcon.cpp
> - Implemented online add/drop primary key
> - Remap server/falcon indexes independently of error condition
> - Renamed 'table' parameter to distinguish between Falcon and server versions
> storage/falcon/ha_falcon.h
> Renamed 'table' parameter to 'srvTable' for TABLE objects passed from server
> === modified file 'mysql-test/suite/falcon/r/falcon_online_index.result'
> --- a/mysql-test/suite/falcon/r/falcon_online_index.result 2008-10-15 06:12:14 +0000
> +++ b/mysql-test/suite/falcon/r/falcon_online_index.result 2009-01-14 18:30:01 +0000
> @@ -131,13 +131,9 @@ Table Non_unique Key_name Seq_in_index C
> t1 0 PRIMARY 1 a NULL 10 NULL NULL BTREE
> #-------- ONLINE: ALTER ADD/DROP PRIMARY KEY --------#
> ALTER ONLINE TABLE t1 DROP PRIMARY KEY;
> -ERROR 42000: This version of MySQL doesn't yet support 'ALTER ONLINE TABLE t1 DROP
> PRIMARY KEY'
> -ALTER TABLE t1 DROP PRIMARY KEY;
> -ALTER ONLINE TABLE t1 ADD PRIMARY KEY (c);
> -ERROR 42000: This version of MySQL doesn't yet support 'ALTER ONLINE TABLE t1 ADD
> PRIMARY KEY (c)'
> SHOW INDEXES FROM t1;
>
> Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_Comment
> -ALTER TABLE t1 ADD PRIMARY KEY (a);
> +ALTER ONLINE TABLE t1 ADD PRIMARY KEY (a);
> #-------- Test: UNIQUE --------#
> ALTER ONLINE TABLE t2 ADD UNIQUE INDEX ix_unique_c (c);
> EXPLAIN SELECT * FROM t2 WHERE c < 25 AND c > 20 ORDER BY c;
>
> === modified file 'mysql-test/suite/falcon/t/falcon_online_index.test'
> --- a/mysql-test/suite/falcon/t/falcon_online_index.test 2008-10-03 05:15:40 +0000
> +++ b/mysql-test/suite/falcon/t/falcon_online_index.test 2009-01-14 18:30:01 +0000
> @@ -194,16 +194,10 @@ SHOW INDEXES FROM t1;
>
> --echo #-------- ONLINE: ALTER ADD/DROP PRIMARY KEY --------#
>
> -# Adding/dropping PRIMARY KEY is not supposed to work ONLINE
> ---error ER_NOT_SUPPORTED_YET
> ALTER ONLINE TABLE t1 DROP PRIMARY KEY;
> -# Now drop primary key (offline) on 'a' temporarily in order to try adding one
> online
> -ALTER TABLE t1 DROP PRIMARY KEY;
> ---error ER_NOT_SUPPORTED_YET
> -ALTER ONLINE TABLE t1 ADD PRIMARY KEY (c);
> SHOW INDEXES FROM t1;
> # Re-set primary key on 'a'
> -ALTER TABLE t1 ADD PRIMARY KEY (a);
> +ALTER ONLINE TABLE t1 ADD PRIMARY KEY (a);
>
> ##
> ## Testing some statement variations using ADD/DROP INDEX
>
> === modified file 'storage/falcon/Database.cpp'
> --- a/storage/falcon/Database.cpp 2009-01-10 16:00:02 +0000
> +++ b/storage/falcon/Database.cpp 2009-01-14 18:30:01 +0000
> @@ -765,7 +765,9 @@ void Database::openDatabase(const char *
> table->setDataSection (sectionId++);
> table->setBlobSection (sectionId++);
>
> - FOR_INDEXES (index, table)
> + // Iterate all indexes, set indexIDs
> +
> + FOR_ALL_INDEXES (index, table)
> index->setIndex (indexId++);
> END_FOR;
> }
>
> === modified file 'storage/falcon/Index.cpp'
> --- a/storage/falcon/Index.cpp 2008-12-08 21:15:06 +0000
> +++ b/storage/falcon/Index.cpp 2009-01-14 18:30:01 +0000
> @@ -614,15 +614,21 @@ void Index::garbageCollect(Record * leav
> {
> int n = 0;
>
> + // Delete index entries of prior record versions of the 'leaving' record
> +
> for (Record *record = leaving; record && record != staying; record =
> record->getGCPriorVersion(), ++n)
> if (record->hasRecord() && record->recordNumber >= 0)
> {
> IndexKey key(this);
> makeKey (record, &key);
>
> - if (!duplicateKey(&key, record->getPriorVersion()) &&
> !duplicateKey (&key, staying))
> + // Delete the index entry for this record version if the key is not used by other
> record versions
> +
> + if (!duplicateKey(&key, record->getPriorVersion()) // key not in 'leaving'
> record chain
> + && !duplicateKey (&key, staying)) // key not in 'staying' record
> chain
> {
> - bool hit = false;
> +
> + // Remove the deferred index entry, if any
>
> if (deferredIndexes.first)
> {
> @@ -630,24 +636,24 @@ void Index::garbageCollect(Record * leav
> sync.lock(Shared);
>
> for (DeferredIndex *deferredIndex = deferredIndexes.first; deferredIndex;
> deferredIndex = deferredIndex->next)
> - if (deferredIndex->deleteNode(&key, record->recordNumber))
> - hit = true;
> + deferredIndex->deleteNode(&key, record->recordNumber);
> }
>
> - if (dbb->deleteIndexEntry(indexId, indexVersion, &key,
> record->recordNumber, TRANSACTION_ID(transaction)))
> - hit = true;
> + // Delete the index entry directly from the index page
> +
> + dbb->deleteIndexEntry(indexId, indexVersion, &key,
> record->recordNumber, TRANSACTION_ID(transaction));
>
> + /***
> if (!hit && !quiet)
> {
> - /***
> Log::log("Index deletion failed for record %d.%d of %s.%s.%s\n",
> record->recordNumber, n, table->schemaName, table->name, (const
> char*) name);
> - ***/
> - //int prevDebug = dbb->debug
> - //dbb->debug = DEBUG_PAGES | DEBUG_KEYS;
> + int prevDebug = dbb->debug
> + dbb->debug = DEBUG_PAGES | DEBUG_KEYS;
> dbb->deleteIndexEntry(indexId, indexVersion, &key,
> record->recordNumber, TRANSACTION_ID(transaction));
> - //dbb->debug = prevDebug ;
> + dbb->debug = prevDebug ;
> }
> + ***/
> }
> }
>
>
> === modified file 'storage/falcon/Statement.cpp'
> --- a/storage/falcon/Statement.cpp 2008-09-03 21:49:18 +0000
> +++ b/storage/falcon/Statement.cpp 2009-01-14 18:30:01 +0000
> @@ -332,15 +332,19 @@ void Statement::createIndex(Syntax * syn
> table->deleteIndex(oldIndex, transaction);
> }
>
> + // If not adding system tables, create and populate the index
> +
> if (!database->formatting)
> {
> Transaction *sysTransaction = database->getSystemTransaction();
> index->create(sysTransaction);
> index->save();
> database->commitSystemTransaction();
> +
> sysTransaction = database->getSystemTransaction();
> table->populateIndex (index, sysTransaction);
> database->commitSystemTransaction();
> +
> database->invalidateCompiledStatements (table);
> //database->flush();
> }
> @@ -1301,13 +1305,32 @@ void Statement::upgradeTable(Syntax * sy
> END_FOR;
>
> FOR_OBJECTS (Field*, field, &changedFields)
> - field->update();
> + field->update(); // does commitSystemTransaction
> END_FOR;
>
> + if (!transaction)
> + transaction = database->getSystemTransaction();
> +
> FOR_OBJECTS (Index*, index, &newIndexes)
> - index->create(transaction);
> - index->save();
> - table->populateIndex (index, transaction);
> + if (!database->formatting)
> + {
> + // Commit and populate the index in separate transactions
> +
> + Transaction *sysTransaction = database->getSystemTransaction();
> + index->create(sysTransaction);
> + index->save();
> + database->commitSystemTransaction();
> +
> + sysTransaction = database->getSystemTransaction();
> + table->populateIndex (index, sysTransaction);
> + database->commitSystemTransaction();
> + }
> + else
> + {
> + index->create(transaction);
> + index->save();
> + table->populateIndex (index, transaction);
> + }
> END_FOR;
>
> for (field = table->fields; field; field = field->next)
>
> === modified file 'storage/falcon/StorageTable.cpp'
> --- a/storage/falcon/StorageTable.cpp 2008-12-10 13:25:17 +0000
> +++ b/storage/falcon/StorageTable.cpp 2009-01-14 18:30:01 +0000
> @@ -194,11 +194,12 @@ int StorageTable::setCurrentIndex(int in
> indexesLocked = true;
> }
>
> - if (!(currentIndex = share->getIndex(indexId)))
> - {
> - clearCurrentIndex();
> - return StorageErrorNoIndex;
> - }
> + currentIndex = share->getIndex(indexId);
> +
> + int ret = checkCurrentIndex();
> +
> + if (ret)
> + return ret;
>
> upperBound = lowerBound = NULL;
> searchFlags = 0;
> @@ -219,6 +220,22 @@ int StorageTable::clearCurrentIndex()
> return 0;
> }
>
> +int StorageTable::checkCurrentIndex()
> +{
> + if (!currentIndex)
> + {
> + clearCurrentIndex();
> +
> + // Use a more benign error until the server protects online alter
> + // with a DDL lock.
> +
> + // return StorageErrorNoIndex;
> + return StorageErrorRecordNotFound;
> + }
> +
> + return 0;
> +}
> +
> int StorageTable::setIndex(StorageIndexDesc* indexDesc)
> {
> return share->setIndex(indexDesc);
> @@ -226,11 +243,10 @@ int StorageTable::setIndex(StorageIndexD
>
> int StorageTable::indexScan(int indexOrder)
> {
> - if (!currentIndex)
> - {
> - clearCurrentIndex();
> - return StorageErrorNoIndex;
> - }
> + int ret = checkCurrentIndex();
> +
> + if (ret)
> + return ret;
>
> int numberSegments = (upperBound) ? upperBound->numberSegments : (lowerBound) ?
> lowerBound->numberSegments : 0;
>
> @@ -267,16 +283,15 @@ void StorageTable::indexEnd(void)
>
> int StorageTable::setIndexBound(const unsigned char* key, int keyLength, int which)
> {
> - if (!currentIndex)
> - {
> - clearCurrentIndex();
> - return StorageErrorNoIndex;
> - }
> + int ret = checkCurrentIndex();
> +
> + if (ret)
> + return ret;
>
> if (which & LowerBound)
> {
> lowerBound = &lowerKey;
> - int ret = storageDatabase->makeKey(currentIndex, key, keyLength, lowerBound);
> + ret = storageDatabase->makeKey(currentIndex, key, keyLength, lowerBound);
>
> if (ret)
> return ret;
> @@ -287,7 +302,7 @@ int StorageTable::setIndexBound(const un
> else if (which & UpperBound)
> {
> upperBound = &upperKey;
> - int ret = storageDatabase->makeKey(currentIndex, key, keyLength, upperBound);
> + ret = storageDatabase->makeKey(currentIndex, key, keyLength, upperBound);
>
> if (ret)
> return ret;
>
> === modified file 'storage/falcon/StorageTable.h'
> --- a/storage/falcon/StorageTable.h 2008-12-10 13:25:17 +0000
> +++ b/storage/falcon/StorageTable.h 2009-01-14 18:30:01 +0000
> @@ -78,6 +78,7 @@ public:
> virtual int indexScan(int indexOrder);
> virtual int setCurrentIndex(int indexId);
> virtual int clearCurrentIndex();
> + virtual int checkCurrentIndex();
> virtual int setIndex(StorageIndexDesc* indexDesc);
> virtual void indexEnd(void);
> virtual int setIndexBound(const unsigned char* key, int keyLength, int which);
>
> === modified file 'storage/falcon/StorageTableShare.cpp'
> --- a/storage/falcon/StorageTableShare.cpp 2009-01-09 22:06:04 +0000
> +++ b/storage/falcon/StorageTableShare.cpp 2009-01-14 18:30:01 +0000
> @@ -410,8 +410,7 @@ int StorageTableShare::dropIndex(Storage
>
> // Remove index description from index mapping
>
> - if (!ret)
> - deleteIndex(indexDesc->id);
> + deleteIndex(indexDesc->id);
>
> return ret;
> }
>
> === modified file 'storage/falcon/Table.cpp'
> --- a/storage/falcon/Table.cpp 2009-01-10 16:00:02 +0000
> +++ b/storage/falcon/Table.cpp 2009-01-14 18:30:01 +0000
> @@ -252,7 +252,9 @@ void Table::create(const char * tableTyp
> dataSectionId = dbb->createSection(TRANSACTION_ID(transaction));
> blobSectionId = dbb->createSection(TRANSACTION_ID(transaction));
>
> - FOR_INDEXES(index, this);
> + // Iterate all indexes, assume indexId == -1
> +
> + FOR_ALL_INDEXES(index, this);
> index->create(transaction);
> END_FOR;
> }
>
> === modified file 'storage/falcon/Table.h'
> --- a/storage/falcon/Table.h 2009-01-08 09:05:26 +0000
> +++ b/storage/falcon/Table.h 2009-01-14 18:30:01 +0000
> @@ -46,7 +46,14 @@ static const int SYNC_VERSIONS_SIZE = 16
> static const int SYNC_THAW_SIZE = 16;
>
> #define FOR_FIELDS(field,table) {for (Field *field=table->fields; field; field =
> field->next){
> -#define FOR_INDEXES(index,table) {for (Index *index=table->indexes; index; index
> = index->next){
> +
> +// For all indexes, regardless of state
> +
> +#define FOR_ALL_INDEXES(index,table) {for (Index *index=table->indexes; index;
> index = index->next){
> +
> +// For all indexes except incomplete or invalid
> +
> +#define FOR_INDEXES(index,table) {for (Index *index=table->indexes; index; index
> = index->next){if (index->indexId == -1) continue;
>
> class Database;
> class Dbb;
>
> === modified file 'storage/falcon/ha_falcon.cpp'
> --- a/storage/falcon/ha_falcon.cpp 2009-01-12 12:32:01 +0000
> +++ b/storage/falcon/ha_falcon.cpp 2009-01-14 18:30:01 +0000
> @@ -67,7 +67,7 @@
> #ifdef DEBUG_BACKLOG
> static const uint LOAD_AUTOCOMMIT_RECORDS = 10000000;
> #else
> -static const uint LOAD_AUTOCOMMIT_RECORDS = 10000;
> +static const uint LOAD_AUTOCOMMIT_RECORDS = 100000;
> #endif
>
> static const char falcon_hton_name[] = "Falcon";
> @@ -553,15 +553,15 @@ int StorageInterface::open(const char *n
>
> thr_lock_data_init((THR_LOCK *)storageShare->impure, &lockData, NULL);
>
> - if (table)
> - mapFields(table);
> + // Map fields for Falcon record encoding
> +
> + mapFields(table);
> +
> + // Map server indexes to Falcon internal indexes
>
> setIndexes(table);
>
> - if (ret)
> - DBUG_RETURN(error(ret));
> -
> - DBUG_RETURN(0);
> + DBUG_RETURN(error(ret));
> }
>
>
> @@ -893,8 +893,12 @@ int StorageInterface::create(const char
> DBUG_RETURN(error(ret));
> }
>
> + // Map fields for Falcon record encoding
> +
> mapFields(form);
>
> + // Map server indexes to Falcon indexes
> +
> setIndexes(table);
>
> DBUG_RETURN(0);
> @@ -908,25 +912,39 @@ int StorageInterface::add_index(TABLE* t
> DBUG_RETURN(ret);
> }
>
> -int StorageInterface::createIndex(const char *schemaName, const char *tableName,
> TABLE *table, int indexId)
> +int StorageInterface::createIndex(const char *schemaName, const char *tableName,
> TABLE *srvTable, int indexId)
> {
> - KEY *key = table->key_info + indexId;
> + int ret = 0;
> + CmdGen gen;
> StorageIndexDesc indexDesc;
> - getKeyDesc(table, indexId, &indexDesc);
> + getKeyDesc(srvTable, indexId, &indexDesc);
>
> - CmdGen gen;
> - const char *unique = (key->flags & HA_NOSAME) ? "unique " : "";
> - gen.gen("create %sindex \"%s\" on %s.\"%s\" ", unique, indexDesc.name, schemaName,
> tableName);
> - genKeyFields(key, &gen);
> - const char *sql = gen.getString();
> + if (indexDesc.primaryKey)
> + {
> + int64 incrementValue = 0;
> + genTable(srvTable, &gen);
> +
> + // Primary keys are a special case, so use upgrade()
> +
> + ret = storageTable->upgrade(gen.getString(), incrementValue);
> + }
> + else
> + {
> + KEY *key = srvTable->key_info + indexId;
> + const char *unique = (key->flags & HA_NOSAME) ? "unique " : "";
> + gen.gen("create %sindex \"%s\" on %s.\"%s\" ", unique, indexDesc.name, schemaName,
> tableName);
> + genKeyFields(key, &gen);
> +
> + ret = storageTable->createIndex(&indexDesc, gen.getString());
> + }
>
> - return storageTable->createIndex(&indexDesc, sql);
> + return ret;
> }
>
> -int StorageInterface::dropIndex(const char *schemaName, const char *tableName, TABLE
> *table, int indexId, bool online)
> +int StorageInterface::dropIndex(const char *schemaName, const char *tableName, TABLE
> *srvTable, int indexId, bool online)
> {
> StorageIndexDesc indexDesc;
> - getKeyDesc(table, indexId, &indexDesc);
> + getKeyDesc(srvTable, indexId, &indexDesc);
>
> CmdGen gen;
> gen.gen("drop index %s.\"%s\"", schemaName, indexDesc.name);
> @@ -1388,9 +1406,14 @@ int StorageInterface::index_read(uchar *
> enum ha_rkey_function find_flag)
> {
> DBUG_ENTER("StorageInterface::index_read");
> - int ret, which = 0;
> + int which = 0;
> ha_statistic_increment(&SSV::ha_read_key_count);
>
> + int ret = storageTable->checkCurrentIndex();
> +
> + if (ret)
> + DBUG_RETURN(error(ret));
> +
> // XXX This needs to be revisited
> switch(find_flag)
> {
> @@ -1456,22 +1479,34 @@ int StorageInterface::index_init(uint id
> nextRecord = 0;
> haveStartKey = false;
> haveEndKey = false;
> - int ret = 0;
>
> - ret = storageTable->setCurrentIndex(idx);
> + // Get and hold a shared lock on StorageTableShare::indexes, then set
> + // the corresponding Falcon index for use on the current thread
> +
> + int ret = storageTable->setCurrentIndex(idx);
>
> + // If the index is not found, remap the index and try again
> +
> if (ret)
> {
> - setIndex(table, idx);
> + storageShare->lockIndexes(true);
> + remapIndexes(table);
> + storageShare->unlockIndexes();
> +
> ret = storageTable->setCurrentIndex(idx);
> - }
>
> - // validateIndexes(table);
> + // Online ALTER allows access to partially deleted indexes, so
> + // fail silently for now to avoid fatal assert in server.
> + //
> + // TODO: Restore error when server imposes DDL lock across the
> + // three phases of online ALTER.
>
> - if (ret)
> - DBUG_RETURN(error(ret));
> -
> - DBUG_RETURN(ret);
> + if (ret)
> + //DBUG_RETURN(error(ret));
> + DBUG_RETURN(error(0));
> + }
> +
> + DBUG_RETURN(error(ret));
> }
>
> int StorageInterface::index_end(void)
> @@ -1526,15 +1561,15 @@ ha_rows StorageInterface::records_in_ran
> DBUG_RETURN(MAX(guestimate, 2));
> }
>
> -void StorageInterface::getKeyDesc(TABLE *table, int indexId, StorageIndexDesc
> *indexDesc)
> +void StorageInterface::getKeyDesc(TABLE *srvTable, int indexId, StorageIndexDesc
> *indexDesc)
> {
> - KEY *keyInfo = table->key_info + indexId;
> + KEY *keyInfo = srvTable->key_info + indexId;
> int numberKeys = keyInfo->key_parts;
>
> indexDesc->id = indexId;
> indexDesc->numberSegments = numberKeys;
> indexDesc->unique = (keyInfo->flags & HA_NOSAME);
> - indexDesc->primaryKey = (table->s->primary_key == (uint)indexId);
> + indexDesc->primaryKey = (srvTable->s->primary_key == (uint)indexId);
>
> // Clean up the index name for internal use
>
> @@ -1586,16 +1621,12 @@ int StorageInterface::rename_table(const
>
> ret = storageShare->renameTable(storageConnection, to);
>
> - if (!ret)
> - remapIndexes(table);
> + remapIndexes(table);
>
> storageShare->unlock();
> storageShare->unlockIndexes();
>
> - if (ret)
> - DBUG_RETURN(error(ret));
> -
> - DBUG_RETURN(ret);
> + DBUG_RETURN(error(ret));
> }
>
> double StorageInterface::read_time(uint index, uint ranges, ha_rows rows)
> @@ -1643,8 +1674,12 @@ int StorageInterface::scanRange(const ke
> haveStartKey = false;
> haveEndKey = false;
> storageTable->upperBound = storageTable->lowerBound = NULL;
> - int ret = 0;
>
> + int ret = storageTable->checkCurrentIndex();
> +
> + if (ret)
> + DBUG_RETURN(error(ret));
> +
> if (start_key && !storageTable->isKeyNull((const unsigned char*)
> start_key->key, start_key->length))
> {
> haveStartKey = true;
> @@ -1655,7 +1690,7 @@ int StorageInterface::scanRange(const ke
> else if (start_key->flag == HA_READ_AFTER_KEY)
> storageTable->setReadAfterKey();
>
> - int ret = storageTable->setIndexBound((const unsigned char*)
> start_key->key,
> + ret = storageTable->setIndexBound((const unsigned char*) start_key->key,
> start_key->length, LowerBound);
> if (ret)
> DBUG_RETURN(error(ret));
> @@ -1708,6 +1743,11 @@ int StorageInterface::index_next(uchar *
> if (activeBlobs)
> freeActiveBlobs();
>
> + int ret = storageTable->checkCurrentIndex();
> +
> + if (ret)
> + DBUG_RETURN(error(ret));
> +
> for (;;)
> {
> lastRecord = storageTable->nextIndexed(nextRecord, lockForUpdate);
> @@ -2332,9 +2372,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 | HA_ADD_UNIQUE_INDEX |
> HA_DROP_UNIQUE_INDEX;
> + supported = supported | HA_ADD_INDEX | HA_DROP_INDEX | HA_ADD_UNIQUE_INDEX |
> HA_DROP_UNIQUE_INDEX | HA_ADD_PK_INDEX | HA_DROP_PK_INDEX;
> /**
> - | HA_ADD_COLUMN | HA_COLUMN_STORAGE | HA_COLUMN_FORMAT | HA_ADD_PK_INDEX |
> HA_DROP_PK_INDEX;
> + | HA_ADD_COLUMN | HA_COLUMN_STORAGE | HA_COLUMN_FORMAT ;
> **/
> HA_ALTER_FLAGS notSupported = ~(supported);
>
> @@ -2367,33 +2407,6 @@ int StorageInterface::check_if_supported
> }
> }
>
> - if (alter_flags->is_set(HA_ADD_INDEX) ||
> alter_flags->is_set(HA_ADD_UNIQUE_INDEX)
> - || alter_flags->is_set(HA_DROP_INDEX) ||
> alter_flags->is_set(HA_DROP_UNIQUE_INDEX))
> - {
> - for (unsigned int n = 0; n < altered_table->s->keys; n++)
> - {
> - KEY *key = altered_table->key_info + n;
> - KEY *tableEnd = table->key_info + table->s->keys;
> - KEY *tableKey;
> -
> - // Determine if this is a new index
> -
> - for (tableKey = table->key_info; tableKey < tableEnd; tableKey++)
> - if (!strcmp(tableKey->name, key->name))
> - break;
> -
> - // Unique, non-null keys are interpreted as primary keys.
> - // Online add/drop primary keys not yet supported.
> -
> - if (tableKey >= tableEnd)
> - if (n == altered_table->s->primary_key)
> - {
> - DBUG_PRINT("info",("Online add/drop primary key not supported"));
> - DBUG_RETURN(HA_ALTER_NOT_SUPPORTED);
> - }
> - }
> - }
> -
> DBUG_RETURN(HA_ALTER_SUPPORTED_NO_LOCK);
> }
>
> @@ -2417,10 +2430,10 @@ int StorageInterface::alter_table_phase2
> 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) ||
> alter_flags->is_set(HA_ADD_UNIQUE_INDEX)) && !ret)
> + if ((alter_flags->is_set(HA_ADD_INDEX) ||
> alter_flags->is_set(HA_ADD_UNIQUE_INDEX) || alter_flags->is_set(HA_ADD_PK_INDEX))
> && !ret)
> ret = addIndex(thd, altered_table, create_info, alter_info, alter_flags);
>
> - if ((alter_flags->is_set(HA_DROP_INDEX) ||
> alter_flags->is_set(HA_DROP_UNIQUE_INDEX)) && !ret)
> + if ((alter_flags->is_set(HA_DROP_INDEX) ||
> alter_flags->is_set(HA_DROP_UNIQUE_INDEX) || alter_flags->is_set(HA_DROP_PK_INDEX))
> && !ret)
> ret = dropIndex(thd, altered_table, create_info, alter_info, alter_flags);
>
> DBUG_RETURN(ret);
> @@ -2503,8 +2516,7 @@ int StorageInterface::addIndex(THD* thd,
>
> // The server indexes may have been reordered, so remap to the Falcon indexes
>
> - if (!ret)
> - remapIndexes(alteredTable);
> + remapIndexes(alteredTable);
>
> storageShare->unlock();
> storageShare->unlockIndexes();
> @@ -2542,8 +2554,7 @@ int StorageInterface::dropIndex(THD* thd
>
> // The server indexes have been reordered, so remap to the Falcon indexes
>
> - if (!ret)
> - remapIndexes(alteredTable);
> + remapIndexes(alteredTable);
>
> storageShare->unlock();
> storageShare->unlockIndexes();
> @@ -2594,26 +2605,25 @@ void StorageInterface::mysqlLogger(int m
> sql_print_information("%s", text);
> }
>
> -int StorageInterface::setIndex(TABLE *table, int indexId)
> +int StorageInterface::setIndex(TABLE *srvTable, int indexId)
> {
> StorageIndexDesc indexDesc;
> - getKeyDesc(table, indexId, &indexDesc);
> + getKeyDesc(srvTable, indexId, &indexDesc);
>
> return storageTable->setIndex(&indexDesc);
> }
>
> -int StorageInterface::setIndexes(TABLE *table)
> +int StorageInterface::setIndexes(TABLE *srvTable)
> {
> int ret = 0;
>
> - if (!table || storageShare->haveIndexes(table->s->keys))
> + if (!srvTable || storageShare->haveIndexes(srvTable->s->keys))
> return ret;
>
> storageShare->lockIndexes(true);
> storageShare->lock(true);
>
> - ret = remapIndexes(table);
> - // validateIndexes(table, true);
> + ret = remapIndexes(srvTable);
>
> storageShare->unlock();
> storageShare->unlockIndexes();
> @@ -2621,41 +2631,47 @@ int StorageInterface::setIndexes(TABLE *
> return ret;
> }
>
> -int StorageInterface::remapIndexes(TABLE *table)
> +// Create an index entry in StorageTableShare for each TABLE index
> +// Assume exclusive lock on StorageTableShare::syncIndexMap
> +
> +int StorageInterface::remapIndexes(TABLE *srvTable)
> {
> int ret = 0;
>
> storageShare->deleteIndexes();
>
> - if (!table)
> + if (!srvTable)
> return ret;
>
> - for (uint n = 0; n < table->s->keys; ++n)
> - if ((ret = setIndex(table, n)))
> - break;
> + // Ok to ignore errors in this context
> +
> + for (uint n = 0; n < srvTable->s->keys; ++n)
> + setIndex(srvTable, n);
>
> + validateIndexes(srvTable, true);
> +
> return ret;
> }
>
> -bool StorageInterface::validateIndexes(TABLE *table, bool exclusiveLock)
> +bool StorageInterface::validateIndexes(TABLE *srvTable, bool exclusiveLock)
> {
> bool ret = true;
>
> - if (!table)
> + if (!srvTable)
> return false;
>
> storageShare->lockIndexes(exclusiveLock);
>
> - for (uint n = 0; (n < table->s->keys) && ret; ++n)
> + for (uint n = 0; (n < srvTable->s->keys) && ret; ++n)
> {
> StorageIndexDesc indexDesc;
> - getKeyDesc(table, n, &indexDesc);
> + getKeyDesc(srvTable, n, &indexDesc);
>
> if (!storageShare->validateIndex(n, &indexDesc))
> ret = false;
> }
>
> - if (ret && (table->s->keys !=
> (uint)storageShare->numberIndexes()))
> + if (ret && (srvTable->s->keys !=
> (uint)storageShare->numberIndexes()))
> ret = false;
>
> storageShare->unlockIndexes();
> @@ -2663,7 +2679,7 @@ bool StorageInterface::validateIndexes(T
> return ret;
> }
>
> -int StorageInterface::genTable(TABLE* table, CmdGen* gen)
> +int StorageInterface::genTable(TABLE* srvTable, CmdGen* gen)
> {
> const char *tableName = storageTable->getName();
> const char *schemaName = storageTable->getSchemaName();
> @@ -2671,9 +2687,9 @@ int StorageInterface::genTable(TABLE* ta
> const char *sep = "";
> char nameBuffer[256];
>
> - for (uint n = 0; n < table->s->fields; ++n)
> + for (uint n = 0; n < srvTable->s->fields; ++n)
> {
> - Field *field = table->field[n];
> + Field *field = srvTable->field[n];
> CHARSET_INFO *charset = field->charset();
>
> if (charset)
> @@ -2692,13 +2708,28 @@ int StorageInterface::genTable(TABLE* ta
> sep = ",\n";
> }
>
> - if (table->s->primary_key < table->s->keys)
> + if (srvTable->s->primary_key < srvTable->s->keys)
> {
> - KEY *key = table->key_info + table->s->primary_key;
> + KEY *key = srvTable->key_info + srvTable->s->primary_key;
> gen->gen(",\n primary key ");
> genKeyFields(key, gen);
> }
>
> +#if 0
> + // Disable until UPGRADE TABLE supports index syntax
> +
> + for (unsigned int n = 0; n < srvTable->s->keys; n++)
> + {
> + if (n != srvTable->s->primary_key)
> + {
> + KEY *key = srvTable->key_info + n;
> + const char *unique = (key->flags & HA_NOSAME) ? "unique " : "";
> + gen->gen(",\n %s key ", unique);
> + genKeyFields(key, gen);
> + }
> + }
> +#endif
> +
> gen->gen (")");
>
> return 0;
> @@ -3739,18 +3770,22 @@ int StorageInterface::recover (handlerto
> DBUG_RETURN(count);
> }
>
> +// Build a record field map for use by encode/decodeRecord()
>
> -void StorageInterface::mapFields(TABLE *table)
> +void StorageInterface::mapFields(TABLE *srvTable)
> {
> + if (!srvTable)
> + return;
> +
> maxFields = storageShare->format->maxId;
> unmapFields();
> fieldMap = new Field*[maxFields];
> memset(fieldMap, 0, sizeof(fieldMap[0]) * maxFields);
> char nameBuffer[256];
>
> - for (uint n = 0; n < table->s->fields; ++n)
> + for (uint n = 0; n < srvTable->s->fields; ++n)
> {
> - Field *field = table->field[n];
> + Field *field = srvTable->field[n];
> storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer), false);
> int id = storageShare->getFieldId(nameBuffer);
>
>
> === modified file 'storage/falcon/ha_falcon.h'
> --- a/storage/falcon/ha_falcon.h 2008-12-08 21:15:06 +0000
> +++ b/storage/falcon/ha_falcon.h 2009-01-14 18:30:01 +0000
> @@ -126,19 +126,19 @@ public:
> int dropIndex(THD* thd, TABLE* alteredTable, HA_CREATE_INFO* createInfo,
> HA_ALTER_INFO* alterInfo, HA_ALTER_FLAGS* alterFlags);
>
> void getDemographics(void);
> - int createIndex(const char *schemaName, const char *tableName, TABLE *table, int
> indexId);
> - int dropIndex(const char *schemaName, const char *tableName, TABLE *table, int
> indexId, bool online);
> - void getKeyDesc(TABLE *table, int indexId, StorageIndexDesc *indexInfo);
> + int createIndex(const char *schemaName, const char *tableName, TABLE *srvTable,
> int indexId);
> + int dropIndex(const char *schemaName, const char *tableName, TABLE *srvTable,
> int indexId, bool online);
> + void getKeyDesc(TABLE *srvTable, int indexId, StorageIndexDesc *indexInfo);
> void startTransaction(void);
> bool threadSwitch(THD *newThread);
> int threadSwitchError(void);
> int error(int storageError);
> void freeActiveBlobs(void);
> - int setIndex(TABLE *table, int indexId);
> - int setIndexes(TABLE *table);
> - int remapIndexes(TABLE *table);
> - bool validateIndexes(TABLE *table, bool exclusiveLock = false);
> - int genTable(TABLE* table, CmdGen* gen);
> + int setIndex(TABLE *srvTable, int indexId);
> + int setIndexes(TABLE *srvTable);
> + int remapIndexes(TABLE *srvTable);
> + bool validateIndexes(TABLE *srvTable, bool exclusiveLock = false);
> + int genTable(TABLE* srvTable, CmdGen* gen);
> int genType(Field *field, CmdGen *gen);
> void genKeyFields(KEY *key, CmdGen *gen);
> void encodeRecord(uchar *buf, bool updateFlag);
>
>