From: Dmitry Lenev Date: July 2 2009 4:47am Subject: bzr commit into mysql-6.1-fk branch (dlenev:2724) Bug#45775 List-Archive: http://lists.mysql.com/commits/77721 X-Bug: 45775 Message-Id: <20090702044722.C21E2B818C@mockturtle> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Boundary_(ID_Ss1Dc+n4zo/OTEswtw7qNQ)" --Boundary_(ID_Ss1Dc+n4zo/OTEswtw7qNQ) MIME-version: 1.0 Content-type: text/plain; CHARSET=US-ASCII Content-transfer-encoding: 7BIT Content-disposition: inline #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); --Boundary_(ID_Ss1Dc+n4zo/OTEswtw7qNQ) MIME-version: 1.0 Content-type: text/bzr-bundle; CHARSET=US-ASCII; name="bzr/dlenev@stripped" Content-transfer-encoding: 7BIT Content-disposition: inline; filename="bzr/dlenev@stripped" # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: dlenev@stripped # target_branch: file:///home/dlenev/src/bzr/mysql-6.1-falcon-bugs/ # testament_sha1: abcdae20c8584d33757a4473cbddfd82db7dd433 # timestamp: 2009-07-02 08:47:22 +0400 # base_revision_id: dlenev@stripped # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWU6lHKwABGrfgEAweXf//3/v /2q////6YAq9XVu1uvjcwCLU9uumh6aDrWW2UFXbdmsNTRNMgEZGk2hpPJpkRp6TQANAAGg0aCUh DBMpkwimyR6h+k0jTQGjTQNGQAGgDUyaaaKZQepphDQAAABo0DQ0aAACREJMk0w1E9RN6eqQ8UyM aMQ0m1NkhoNNADHGjJkYRiAYTQYBNBoGTJoyZDCAwkiEAJkNDSYTImBNT0T0mk9JtE9T1NAZGGpp gJPj+UEjGMGz/IgG022mNp47xSF7tOQduNavdd8lJBy/tjE940RGjwtvYceBuMnCNEkExZ+mgRNR 0PPt/Ct4Z1xtva1Lv4Hs08tbBvtCzNUZLS1PPy5+BiSNKSZlfAQoanNxK9mgDKzALDsFk5eV9/3h +GgdSDiG7oI+AbgbbYNoGy74gf3S9mXemtCO/xSZWsf1RiEYGLkSPOKckic5xWcFeGkT5mhGQD1y LdjSD69zgyzS/NFpilS5br+A3mJ4eeKU1fkzA8XbBS+FNRpRtNpjOywFQFOtVLtDnjzJn8tsg+Sl kjNB/WVO1zXBVo0dVha6L64OKkYnWqjc5XECBHSqIZKCISWtAVrccZgKWhMiKo12rnm3KlzzrClR Q0mprEcLb52LaBvrqVfffhTSq5xWp4HtOOLCbcRnD1EtIatm5s0u5kfoqmrgzpPoGsQjYBbUoGjz 82mnlyPnBTzY7SjGJD6hmXWQtwxtMx3S+6ODFA/dh0a3Z51uth07wwrkLMmJw7ZCkg2q6gVWMswu AgRYkGrVnvNRgDV8GLQjoXEfwnKnDpzSNYLYEgoKCuaWPrt2TfTTqtObT6VYwA6cGTH7b4eaPA+1 jaifCYPxRuC02k6EuUHaA0NYwkodHM2uogSoTxrUFkWZUpfTxEkyhRYaSlI3DSCQM6DYFIl8Dl2G 8lgEZPhQVMYMYvPjIdxiyqzgelil3UQ7EEkpFtU++d90IODoqUUMWNQcx9BLO4mKXICczFdwKBKV EhsGrwuMZrHgjMcNBArOsGZBhg95Giui9Rhhj50R70F6ZIHAqhQhU4Tjkzo9Q0ibTOEYM4Mpz+B7 myVFBQlXmPIkVESoF0Bie+y7EayBVTF8HmFpk4XktxSX8B6s6ffUORZs1udyRQVEwSIGrtPQF9xs V1WmDUlz55aWtkr5nROEXkBRZJokgUT5QwLxyeB38SwFzlOWTGmIUwghyQ5IpLyk2Gs9OZNzi3UP SrBiPx0wL5X7B99VFBaRkBXNM6OZlGNdjMRw0EKbo1bIqBoGMaBoEwS3+Q2Zyd8c07LlKspisEUU rMDHYVJGjG0uOMfPeMBIWj9Kyo7hhnGm4usdYMrmakRFigFOQRh7cibr3IojIFiyl+JcViMKCkYW A80mSbTUI01k3Zs9911ESxiTmZG5jduNBTCg1byNhjK4pLBzgPNPow7z1YVrYNquGAV11YxhIDtW 1800uppLbxWMXESkh122BCp6rWVW9kXid5WorSmbf6VjFqfIHJ9RRoOxnN962dTWByswOFQ/rgh7 vODIGgu8XOtmqCbSXOOOjS8NMumUlPaGe74C4Vgmxtv3gu2ohI31fvp2u+Sm1GFPQCQcIOC+pxwA G/FoAfyMbfrsbRFh9QRzGFkPzTKl5fpGDdHo5O2JkibOUf9WNqwzm6KESt/OUF0up7+uibUGDUw6 qxhQm6xLk1qmsXq1fuFAZC9SqCCJIGSYht/vJRZFupgO/L3POioaOc/Ep6ypvlTPd7D0nbIvUdlS 3QM5QBpb3Ts6B8hxi+zA1kRC5tgUj/sitI/Kz6H8OUluC3o62QZ5izW5nPsfb+TxFnBTAlD5nsXz yrUleT0jH21YX0KID6zky3i05y0iLPQdB7jYUjjX0mJGgYLMyYZMQL1Gr09NR427wx9ZaToRn7fm Ymcixr1m7pPE7jIrXQY5jzKj+27A6Nf3nj3Z3yszeynlMw3ZMfMEMcABlUQKGDIiRnoKYOfB3SZ1 uCHI9Rkazp4GUQTkRyoXEJhe0hcCrNi6+gwKjif4BXlZ2EyWM5ZB4woEwBkPGDNhfCNWTAPAHBiA dkxiF9uIT0QToxNZ6SSEXUEwnjvkTzONFOKd5TCeIcTT0GQ/Lx0Q6tJYUEjwLy6D3VGph0dhTp8E QSzoZihx0wMhkSJWYN0G0YOlmqlilFFnZdkUDy7vjN0+KaPKfHiwVV8NQBnnC3hSoAWhgaLJFrPR lGMRxpDk52BBaa7OBA9vReGsJlREMEajYgIwSM6MS9B2/LpTMmGBMhgZlXuC9eXRyQeB4mq49ZiR MPxD2MLqOxFyPMb/QYhrRk1L9TTgmiV+jh7MiIUcpwhnLbjFJEmDSC4goBI5h3lX5qRLtOHdz6h3 HOkUkCMOjEbXcrU9wzMutobH3f+tWseSDG6+dxXVP5HedIlPvi2vX6+sFa/Cg81ah+rQeTzIVgJS KTtoj8mJZLidKcDtJlyQizdXRncChC9C2hN+35sjGRtRCN5huRuWnoP1qMzGAarDuBeIL3G/MeRI PPccy2UYGsqU9T4V4BpsTY2P9WSNJnsL7jXi1rEjEVcQqAY8gMQwYho4ACA2eGa98bGVGXnbG4tc pBROeZqJDE++DuyQQgtwZEHwBKpKeWYtBMzV83A4vOWOkJkC7CO1C4IVA01a/6iBr4NTUFiM8Cki 0iMzAHB0MyGKfsPRdhf1SpBNV49kyAgSFJwc+upf5aNiOr31bSRSFaGoteCGGAssK4rlYxrZM4L3 kH2kAWrrc9pmN5JhI2jzjBjDuNkQJiLeibwiDiM1Rho075KIIE6JqPlFV2zzYx0hQTDGqYc0IUNg 1vSMHV1cxQstnRYG9QezaCn0hDN0sG6jROI5/W59PwcwkOaNA2s+5ee9m88hj+ZiG3y3aTeJECo5 ot7inI+o4Eg6Gfk/8mLhoo8AZApbI41djVTSngk/FonjNIFz5foxw+FwcWUKztBiukS/2hkG7HLO EAnw8KaA0IpW780QEDMkmvSVpwiGxA9xPl47vp1GZOaPu7VMcSJSDu42YoRC4jUtnJybE6e4mU7S JiKNSKhN5VIoGAr7BmOxHE22242IR1n+jksOhwoUIMlNYEBjEBWrSowZsjjW4JATFTRkXyfRxaRe kImJIiBrh3ZjS1QWyJ4xh7ynaxM0wFmhOjvYTvVkWk3jkMdFYqLXZdoDSVU1QgwlEF7FN02CbAWQ LGyfhzBaoZ0wmY6CCcLXniBbZKCTl7adhmnC8o1P7q9Og2JwDFL0WzSoKOil0jgRPieNBy0H4Z/s u9sAwyLzUKJBHsPeZE5lSGbYYpYriXAsTnRmLCNAxG266WOYIUCGa3R7mkVivgKWkQ4wDDSKjjI4 FaEQoDMaCU/xijy20WFucYHBHSYlp6k4ThP+LuSKcKEgnUo5WA== --Boundary_(ID_Ss1Dc+n4zo/OTEswtw7qNQ)--