List:Commits« Previous MessageNext Message »
From:Christopher Powers Date:June 28 2009 6:58pm
Subject:bzr commit into mysql-6.0-falcon-team branch (christopher.powers:2750)
Bug#45775
View as plain text  
#At file:///home/cpowers/work/dev/dev-04/mysql/

 2750 Christopher Powers	2009-06-28
      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.
      added:
        mysql-test/suite/falcon/r/falcon_bug_45775.result
        mysql-test/suite/falcon/t/falcon_bug_45775.test
      modified:
        storage/falcon/StorageTableShare.cpp
        storage/falcon/StorageTableShare.h
        storage/falcon/ha_falcon.cpp
        storage/falcon/ha_falcon.h

per-file messages:
  mysql-test/suite/falcon/r/falcon_bug_45775.result
    Result file for falcon_bug_45775.test
  mysql-test/suite/falcon/t/falcon_bug_45775.test
    Test case to confirm that Falcon correctly distinguishes between
    a primary key and a unique key with non-null columns.
  storage/falcon/StorageTableShare.cpp
    StorageTableShare::upgrade()
    
    If StorageTableShare::upgradeTable() failed, then StorageTable::table
    was set to NULL, causing downstream exception.
    
    Renamed StorageErrorTableExits to StorageErrorTableExists.
  storage/falcon/StorageTableShare.h
    Renamed StorageErrorTableExits to StorageErrorTableExists.
  storage/falcon/ha_falcon.cpp
    Added StorageInterface::isPrimaryKey() to definitively identify
    the primary key in TABLE.
  storage/falcon/ha_falcon.h
    Added StorageInterface::isPrimaryKey()
=== added file 'mysql-test/suite/falcon/r/falcon_bug_45775.result'
--- a/mysql-test/suite/falcon/r/falcon_bug_45775.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/falcon/r/falcon_bug_45775.result	2009-06-28 18:58:38 +0000
@@ -0,0 +1,31 @@
+*** Bug #45775 ***
+SET @@storage_engine = 'Falcon';
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a int NOT NULL UNIQUE, b int, c int);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) NOT NULL,
+  `b` int(11) DEFAULT NULL,
+  `c` int(11) DEFAULT NULL,
+  UNIQUE KEY `a` (`a`)
+) ENGINE=Falcon DEFAULT CHARSET=latin1
+ALTER TABLE t1 ADD PRIMARY KEY (b);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) NOT NULL,
+  `b` int(11) NOT NULL DEFAULT '0',
+  `c` int(11) DEFAULT NULL,
+  PRIMARY KEY (`b`),
+  UNIQUE KEY `a` (`a`)
+) ENGINE=Falcon DEFAULT CHARSET=latin1
+INSERT INTO t1 VALUES (1,1,1);
+INSERT INTO t1 VALUES (1,1,2);
+ERROR 23000: Duplicate entry '1' for key 'a'
+INSERT INTO t1 VALUES (1,2,2);
+ERROR 23000: Duplicate entry '1' for key 'a'
+INSERT INTO t1 VALUES (2,1,2);
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+INSERT INTO t1 VALUES (2,2,2);
+DROP TABLE t1;

=== added file 'mysql-test/suite/falcon/t/falcon_bug_45775.test'
--- a/mysql-test/suite/falcon/t/falcon_bug_45775.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/falcon/t/falcon_bug_45775.test	2009-06-28 18:58:38 +0000
@@ -0,0 +1,50 @@
+#
+# Bug #45775: Crash when adding primary key to Falcon table with unique constraint
+#
+# The server treats a primary key and a unique key with non-null columns as
+# effectively the same. This test confirms that the storage engine makes the
+# correct distinction between the two.
+#
+--echo *** Bug #45775 ***
+
+--source include/have_falcon.inc
+
+# ----------------------------------------------------- #
+# --- Initialisation                                --- #
+# ----------------------------------------------------- #
+let $engine = 'Falcon';
+eval SET @@storage_engine = $engine;
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+# ----------------------------------------------------- #
+# --- Test                                          --- #
+# ----------------------------------------------------- #
+CREATE TABLE t1 (a int NOT NULL UNIQUE, b int, c int);
+SHOW CREATE TABLE t1;
+ALTER TABLE t1 ADD PRIMARY KEY (b);
+SHOW CREATE TABLE t1;
+
+INSERT INTO t1 VALUES (1,1,1);
+
+## Should fail with key conflict
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (1,1,2);
+
+## Should fail with unique key conflict
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (1,2,2);
+
+## Should fail with primary key conflict
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (2,1,2);
+
+INSERT INTO t1 VALUES (2,2,2);
+
+# ----------------------------------------------------- #
+# --- Final cleanup                                 --- #
+# ----------------------------------------------------- #
+DROP TABLE t1;
+

=== modified file 'storage/falcon/StorageTableShare.cpp'
--- a/storage/falcon/StorageTableShare.cpp	2009-04-27 16:43:53 +0000
+++ b/storage/falcon/StorageTableShare.cpp	2009-06-28 18:58:38 +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-06-24 22:17:25 +0000
+++ b/storage/falcon/StorageTableShare.h	2009-06-28 18:58:38 +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-06-24 22:17:25 +0000
+++ b/storage/falcon/ha_falcon.cpp	2009-06-28 18:58:38 +0000
@@ -91,6 +91,10 @@ extern StorageHandler	*storageHandler;
 #undef PARAMETER_UINT
 #undef PARAMETER_BOOL
 
+// Unique name assigned by server to the primary key
+
+extern const char *primary_key_name;
+
 ulonglong	falcon_record_memory_max;
 ulonglong	falcon_serial_log_file_size;
 uint		falcon_allocation_extent;
@@ -909,16 +913,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));
 				}
 
@@ -1586,6 +1588,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));
+}
+
 void StorageInterface::getKeyDesc(TABLE *srvTable, int indexId, StorageIndexDesc *indexDesc)
 {
 	KEY *keyInfo = srvTable->key_info + indexId;
@@ -1594,7 +1609,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
 	
@@ -2151,7 +2166,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);
 
@@ -2557,10 +2572,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);
 	
@@ -2815,7 +2826,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-06-24 21:28:30 +0000
+++ b/storage/falcon/ha_falcon.h	2009-06-28 18:58:38 +0000
@@ -146,6 +146,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);

Thread
bzr commit into mysql-6.0-falcon-team branch (christopher.powers:2750)Bug#45775Christopher Powers28 Jun