List:Commits« Previous MessageNext Message »
From:Kevin Lewis Date:January 15 2009 5:35am
Subject:Re: bzr commit into mysql-6.0-falcon-team branch (cpowers:2957)
Bug#42099
View as plain text  
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);
> 
> 
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