bzr commit into mysql-6.1-fk branch (dlenev:2724) Bug#45775

[Dmitry Lenev]

#At file:///home/dlenev/src/bzr/mysql-6.1-falcon-bugs/ based on revid:dlenev@stripped

 2724 Dmitry Lenev	2009-07-02
      Bug #45775, "Crash when adding primary key to Falcon table with unique constraint."
      
      There are several issues here:
      
      1) The server assigns TABLE->s->primary_key != MAX_KEY for EITHER a primary key
         OR a unique key with non-null columns.
      
      2) Falcon assumed ::primary_key unambiguously identifies ONLY the primary key,
         and therfore incorrectly interpreted (a INT NOT NULL UNIQUE) as being a
         primary key declaration.
      
      3) Given (2), the command "ALTER TABLE ADD PRIMARY KEY (b)" caused an update
         conflict for which Falcon raised an exception that subsequently crashed.
      
      4) The crash was caused by a null pointer encountered during exception handling.
      
      Item 4 was easy to fix. Item 2, not so easy.
      
      Fortunately, the server always names the primary key "PRIMARY", so to
      distinguish between a primary key and a unique, non-null key, Falcon now checks
      the key name.
      
      Manually applied patch from Falcon team tree to mysql-6.1-fk tree.

    modified:
      storage/falcon/StorageTableShare.cpp
      storage/falcon/StorageTableShare.h
      storage/falcon/ha_falcon.cpp
      storage/falcon/ha_falcon.h
=== modified file 'storage/falcon/StorageTableShare.cpp'
--- a/storage/falcon/StorageTableShare.cpp	2009-02-04 18:17:01 +0000
+++ b/storage/falcon/StorageTableShare.cpp	2009-07-02 04:47:16 +0000
@@ -219,11 +219,11 @@ int StorageTableShare::create(StorageCon
 			case TABLESPACE_NOT_EXIST_ERROR:
 				return StorageErrorTableSpaceNotExist;
 			default:
-				return StorageErrorTableExits;
+				return StorageErrorTableExists;
 			}
 		}
 	if (!table)
-		return StorageErrorTableExits;
+		return StorageErrorTableExists;
 	
 	format = table->getCurrentFormat();
 
@@ -235,8 +235,12 @@ int StorageTableShare::create(StorageCon
 
 int StorageTableShare::upgrade(StorageConnection *storageConnection, const char* sql, int64 autoIncrementValue)
 {
-	if (!(table = storageDatabase->upgradeTable(storageConnection, name, schemaName, sql, autoIncrementValue)))
-		return StorageErrorTableExits;
+	Table* tableUpgrade = storageDatabase->upgradeTable(storageConnection, name, schemaName, sql, autoIncrementValue);
+	
+	if (tableUpgrade)
+		table = tableUpgrade;
+	else
+		return StorageErrorTableExists;
 
 	format = table->getCurrentFormat();
 	

=== modified file 'storage/falcon/StorageTableShare.h'
--- a/storage/falcon/StorageTableShare.h	2009-03-28 18:10:17 +0000
+++ b/storage/falcon/StorageTableShare.h	2009-07-02 04:47:16 +0000
@@ -100,7 +100,7 @@ enum StorageError {
 	StorageErrorTableNotFound		= -3,
 	StorageErrorNoIndex				= -4,
 	StorageErrorBadKey				= -5,
-	StorageErrorTableExits			= -6,
+	StorageErrorTableExists			= -6,
 	StorageErrorNoSequence			= -7,
 	StorageErrorUpdateConflict		= -8,
 	StorageErrorUncommittedUpdates	= -9,		// specific for drop table

=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp	2009-05-14 21:49:53 +0000
+++ b/storage/falcon/ha_falcon.cpp	2009-07-02 04:47:16 +0000
@@ -90,6 +90,10 @@ extern StorageHandler	*storageHandler;
 #undef PARAMETER_UINT
 #undef PARAMETER_BOOL
 
+// Unique name assigned by server to the primary key
+
+extern const LEX_STRING primary_key_name;
+
 ulonglong	falcon_record_memory_max;
 ulonglong	falcon_serial_log_file_size;
 uint		falcon_allocation_extent;
@@ -908,16 +912,14 @@ int StorageInterface::create(const char 
 	if ((ret = storageTable->create(gen.getString(), incrementValue)))
 		{
 		storageTable->deleteTable();
-		
 		DBUG_RETURN(error(ret));
 		}
 
 	for (n = 0; n < form->s->keys; ++n)
-		if (n != form->s->primary_key)
+		if (!isPrimaryKey(form, n))
 			if ((ret = createIndex(schemaName, tableName, form, n)))
 				{
 				storageTable->deleteTable();
-
 				DBUG_RETURN(error(ret));
 				}
 
@@ -1585,6 +1587,19 @@ ha_rows StorageInterface::records_in_ran
 	DBUG_RETURN(MAX(guestimate, 2));
 }
 
+bool StorageInterface::isPrimaryKey(TABLE *srvTable, uint indexId)
+{
+	uint primaryKey = srvTable->s->primary_key;
+	
+	// If primary_key != MAX_KEY, then the key is either a primary key or
+	// a unique key with all non-NULL columns. Check the key name to
+	// distinguish between them. The primary key is always "PRIMARY". 
+	
+	return (indexId == primaryKey &&
+			primaryKey != MAX_KEY &&
+			!strcmp(srvTable->s->key_info[primaryKey].name, primary_key_name.str));
+}
+
 void StorageInterface::getKeyDesc(TABLE *srvTable, int indexId, StorageIndexDesc *indexDesc)
 {
 	KEY *keyInfo = srvTable->key_info + indexId;
@@ -1593,7 +1608,7 @@ void StorageInterface::getKeyDesc(TABLE 
 	indexDesc->id			  = indexId;
 	indexDesc->numberSegments = numberKeys;
 	indexDesc->unique		  = (keyInfo->flags & HA_NOSAME);
-	indexDesc->primaryKey	  = (srvTable->s->primary_key == (uint)indexId);
+	indexDesc->primaryKey	  = isPrimaryKey(srvTable, indexId);
 	
 	// Clean up the index name for internal use
 	
@@ -2150,7 +2165,7 @@ int StorageInterface::getMySqlError(int 
 			DBUG_PRINT("info", ("StorageErrorBadKey"));
 			return (HA_ERR_WRONG_INDEX);
 
-		case StorageErrorTableExits:
+		case StorageErrorTableExists:
 			DBUG_PRINT("info", ("StorageErrorTableExits"));
 			return (HA_ERR_TABLE_EXIST);
 
@@ -2552,10 +2567,6 @@ int StorageInterface::addColumn(THD* thd
 {
 	int ret;
 	int64 incrementValue = 0;
-	/***
-	const char *tableName = storageTable->getName();
-	const char *schemaName = storageTable->getSchemaName();
-	***/
 	CmdGen gen;
 	genTable(alteredTable, &gen);
 	
@@ -2808,7 +2819,9 @@ int StorageInterface::genTable(TABLE* sr
 		sep = ",\n";
 		}
 
-	if (srvTable->s->primary_key < srvTable->s->keys)
+	// Confirm that primary_key is actually a primary key
+		
+	if (isPrimaryKey(srvTable, srvTable->s->primary_key))
 		{
 		KEY *key = srvTable->key_info + srvTable->s->primary_key;
 		gen->gen(",\n  primary key ");

=== modified file 'storage/falcon/ha_falcon.h'
--- a/storage/falcon/ha_falcon.h	2009-01-15 20:29:54 +0000
+++ b/storage/falcon/ha_falcon.h	2009-07-02 04:47:16 +0000
@@ -129,6 +129,7 @@ public:
 	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);
+	bool			isPrimaryKey(TABLE *srvTable, uint indexId);
 	void			startTransaction(void);
 	bool			threadSwitch(THD *newThread);
 	int				threadSwitchError(void);


Attachment: [text/bzr-bundle] bzr/[e-mail]