MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Christopher Powers Date:January 14 2009 6:30pm
Subject:bzr commit into mysql-6.0-falcon-team branch (cpowers:2957) Bug#42099
View as plain text  
#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);

Thread
bzr commit into mysql-6.0-falcon-team branch (cpowers:2957) Bug#42099Christopher Powers14 Jan
  • Re: bzr commit into mysql-6.0-falcon-team branch (cpowers:2957)Bug#42099Kevin Lewis15 Jan
    • Re: bzr commit into mysql-6.0-falcon-team branch (cpowers:2957)Bug#42099Christopher Powers15 Jan
  • Re: bzr commit into mysql-6.0-falcon-team branch (cpowers:2957)Bug#42099John Embretsen15 Jan
    • Re: bzr commit into mysql-6.0-falcon-team branch (cpowers:2957)Bug#42099Christopher Powers17 Jan