Skip to content

Commit

Permalink
Change the way index deletion works (part of #2811)
Browse files Browse the repository at this point in the history 
* Instead of trying to figure out if an index is used by an FK ahead of time Sequel Pro will now simply run the query and check for the error code (and only for error 1553 will it attempt to also remove the FK)
* This means that a user will receive two remove dialogs in this rare case, but I think that is actually preferable. Common wisdom shows that users never read the first warning dialog, so in the past they may have agreed to something that they didn’t intend to do. The second dialog should actually make them pause and read it. Also there is a different confirmation button now.
* This also fixes the code to detect which FK in particular MySQL is referring to. SP should now correctly handle compound indexes and multi-column FKs as well as ambiguous results.
  • Loading branch information
@dmoagx
dmoagx committed Jun 6, 2017
1 parent 50e309b commit e52546b
Showing 1 changed file with 104 additions and 60 deletions.
164 changes: 104 additions & 60 deletions Source/SPIndexesController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#import "SPTableStructure.h"
#import "SPTableStructureLoading.h"
#import "SPThreadAdditions.h"
#import "SPFunctions.h"

#import <SPMySQL/SPMySQL.h>

Expand Down Expand Up @@ -245,34 +246,15 @@ - (IBAction)removeIndex:(id)sender

if ((index == -1) || (index > ((NSInteger)[indexes count] - 1))) return;

NSString *keyName = [[indexes objectAtIndex:index] objectForKey:@"Key_name"];
NSString *columnName = [[indexes objectAtIndex:index] objectForKey:@"Column_name"];

BOOL hasForeignKey = NO;
NSString *constraintName = @"";

// Check to see whether the user is attempting to remove an index that a foreign key constraint depends on
// thus would result in an error if not dropped before removing the index.
for (NSDictionary *constraint in [tableData getConstraints])
{
for (NSString *column in [constraint objectForKey:@"columns"])
{
if ([column isEqualToString:columnName]) {
hasForeignKey = YES;
constraintName = [constraint objectForKey:@"name"];
break;
}
}
}
NSString *keyName = [[indexes objectAtIndex:index] objectForKey:@"Key_name"];

NSString *info = hasForeignKey ? [NSString stringWithFormat:NSLocalizedString(@"The foreign key relationship '%@' has a dependency on this index. This relationship must be removed before the index can be deleted.\n\nAre you sure you want to continue to delete the relationship and the index? This action cannot be undone.", @"delete index and foreign key informative message"), constraintName]
: [NSString stringWithFormat:NSLocalizedString(@"Are you sure you want to delete the index '%@'? This action cannot be undone.", @"delete index informative message"), keyName];
if(![keyName length]) return; //safeguard for the contextInfo array creation below

NSAlert *alert = [NSAlert alertWithMessageText:[NSString stringWithFormat:NSLocalizedString(@"Delete index '%@'?", @"delete index message"), keyName]
defaultButton:NSLocalizedString(@"Delete", @"delete button")
alternateButton:NSLocalizedString(@"Cancel", @"cancel button")
otherButton:nil
informativeTextWithFormat:@"%@", info];
informativeTextWithFormat:NSLocalizedString(@"Are you sure you want to delete the index '%@'? This action cannot be undone.", @"delete index informative message"), keyName];

[alert setAlertStyle:NSCriticalAlertStyle];

Expand All @@ -286,7 +268,7 @@ - (IBAction)removeIndex:(id)sender
[alert beginSheetModalForWindow:[dbDocument parentWindow]
modalDelegate:self
didEndSelector:@selector(removeIndexSheetDidEnd:returnCode:contextInfo:)
contextInfo:(hasForeignKey) ? @"removeIndexAndForeignKey" : @"removeIndex"];
contextInfo:[@{@"Key_name" : keyName} retain]]; // contextInfo is NOT retained by Cocoa!
}

/**
Expand Down Expand Up @@ -639,22 +621,19 @@ - (void)removeIndexSheetDidEnd:(NSAlert *)alert returnCode:(NSInteger)returnCode
{
// Order out current sheet to suppress overlapping of sheets
[[alert window] orderOut:nil];

NSDictionary *info = [(id)contextInfo autorelease]; //we explicitly retained it beforehand, because Cocoa does NOT!

if (returnCode == NSAlertDefaultReturn) {
[dbDocument startTaskWithDescription:NSLocalizedString(@"Removing index...", @"removing index task status message")];

NSMutableDictionary *indexDetails = [NSMutableDictionary dictionary];

[indexDetails setObject:[indexes objectAtIndex:[indexesTableView selectedRow]] forKey:@"Index"];
[indexDetails setObject:[NSNumber numberWithBool:[(NSString *)contextInfo hasSuffix:@"AndForeignKey"]] forKey:@"RemoveForeignKey"];

if ([NSThread isMainThread]) {
[NSThread detachNewThreadWithName:SPCtxt(@"SPIndexesController index removal thread", dbDocument) target:self selector:@selector(_removeIndexUsingDetails:) object:indexDetails];
[NSThread detachNewThreadWithName:SPCtxt(@"SPIndexesController index removal thread", dbDocument) target:self selector:@selector(_removeIndexUsingDetails:) object:info];

[dbDocument enableTaskCancellationWithTitle:NSLocalizedString(@"Cancel", @"cancel button") callbackObject:self callbackFunction:NULL];
}
else {
[self _removeIndexUsingDetails:indexDetails];
[self _removeIndexUsingDetails:info];
}
}
}
Expand Down Expand Up @@ -914,58 +893,48 @@ - (void)_removeIndexUsingDetails:(NSDictionary *)indexDetails
{
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

NSDictionary *index = [indexDetails objectForKey:@"Index"];
BOOL removeForeignKey = [[indexDetails objectForKey:@"RemoveForeignKey"] boolValue];
NSString *index = [indexDetails objectForKey:@"Key_name"];
NSString *fkName = [indexDetails objectForKey:@"ForeignKey"];

// Remove the foreign key dependency before the index if required
if (removeForeignKey) {

NSString *columnName = [index objectForKey:@"Column_name"];

NSString *constraintName = @"";

// Check to see whether the user is attempting to remove an index that a foreign key constraint depends on
// thus would result in an error if not dropped before removing the index.
for (NSDictionary *constraint in [tableData getConstraints])
{
for (NSString *column in [constraint objectForKey:@"columns"])
{
if ([column isEqualToString:columnName]) {
constraintName = [constraint objectForKey:@"name"];
break;
}
}
}
if ([fkName length]) {

[connection queryString:[NSString stringWithFormat:@"ALTER TABLE %@ DROP FOREIGN KEY %@", [table backtickQuotedString], [constraintName backtickQuotedString]]];
[connection queryString:[NSString stringWithFormat:@"ALTER TABLE %@ DROP FOREIGN KEY %@", [table backtickQuotedString], [fkName backtickQuotedString]]];

// Check for errors, but only if the query wasn't cancelled
if ([connection queryErrored] && ![connection lastQueryWasCancelled]) {
NSMutableDictionary *errorDictionary = [NSMutableDictionary dictionary];

[errorDictionary setObject:NSLocalizedString(@"Unable to delete relation", @"error deleting relation message") forKey:@"title"];
[errorDictionary setObject:[NSString stringWithFormat:NSLocalizedString(@"An error occurred while trying to delete the relation '%@'.\n\nMySQL said: %@", @"error deleting relation informative message"), constraintName, [connection lastErrorMessage]] forKey:@"message"];
[errorDictionary setObject:[NSString stringWithFormat:NSLocalizedString(@"An error occurred while trying to delete the relation '%@'.\n\nMySQL said: %@", @"error deleting relation informative message"), fkName, [connection lastErrorMessage]] forKey:@"message"];

[(SPTableStructure*)[tableStructure onMainThread] showErrorSheetWith:errorDictionary];
}
}

if ([[index objectForKey:@"Key_name"] isEqualToString:@"PRIMARY"]) {
if ([index isEqualToString:@"PRIMARY"]) {
[connection queryString:[NSString stringWithFormat:@"ALTER TABLE %@ DROP PRIMARY KEY", [table backtickQuotedString]]];
}
else {
[connection queryString:[NSString stringWithFormat:@"ALTER TABLE %@ DROP INDEX %@",
[table backtickQuotedString], [[index objectForKey:@"Key_name"] backtickQuotedString]]];
[table backtickQuotedString], [index backtickQuotedString]]];
}

// Check for errors, but only if the query wasn't cancelled
if ([connection queryErrored] && ![connection lastQueryWasCancelled]) {
NSMutableDictionary *errorDictionary = [NSMutableDictionary dictionary];

[errorDictionary setObject:NSLocalizedString(@"Unable to delete index", @"error deleting index message") forKey:@"title"];
[errorDictionary setObject:[NSString stringWithFormat:NSLocalizedString(@"An error occured while trying to delete the index.\n\nMySQL said: %@", @"error deleting index informative message"), [connection lastErrorMessage]] forKey:@"message"];

[(SPTableStructure*)[tableStructure onMainThread] showErrorSheetWith:errorDictionary];
//if the last error was 1553 and we did not already try to remove a FK beforehand, we have to request to remove the foreign key before we can remove the index
if([connection lastErrorID] == 1553 /* ER_DROP_INDEX_FK */ && ![fkName length]) {
NSDictionary *details = @{@"Key_name": index, @"error": SPBoxNil([connection lastErrorMessage])};
[self performSelectorOnMainThread:@selector(_removingIndexFailedWithForeignKeyError:) withObject:details waitUntilDone:NO];
}
else {
NSMutableDictionary *errorDictionary = [NSMutableDictionary dictionary];

[errorDictionary setObject:NSLocalizedString(@"Unable to delete index", @"error deleting index message") forKey:@"title"];
[errorDictionary setObject:[NSString stringWithFormat:NSLocalizedString(@"An error occured while trying to delete the index.\n\nMySQL said: %@", @"error deleting index informative message"), [connection lastErrorMessage]] forKey:@"message"];

[(SPTableStructure*)[tableStructure onMainThread] showErrorSheetWith:errorDictionary];
}
}
else {
[tableData resetAllData];
Expand All @@ -979,6 +948,81 @@ - (void)_removeIndexUsingDetails:(NSDictionary *)indexDetails
[pool drain];
}

/**
* If removing an index failed, because an FK depends on it (mysql error 1553) this
* will ask the user to confirm deleting the FK, too (if it is found).
*
* MUST be called on the UI thread!
*/
- (void)_removingIndexFailedWithForeignKeyError:(NSDictionary *)info
{
NSString *keyName = [info objectForKey:@"Key_name"];

//we have to find out which fk uses this index (and need to watch out for compound indexes)
NSString *constraintName = nil;

NSMutableArray *myColumns = [NSMutableArray array];

for (NSDictionary *indexPart in indexes) {
if ([[indexPart objectForKey:@"Key_name"] isEqualToString:keyName]) {
[myColumns addObject:[indexPart objectForKey:@"Column_name"]];
}
}

//if the index has no columns, something's fucky
if(![myColumns count]) {
SPOnewayAlertSheet(
[NSString stringWithFormat:NSLocalizedString(@"Failed to remove index '%@'", @"table structure : indexes : delete index : no columns error : title"),keyName],
[dbDocument parentWindow],
NSLocalizedString(@"Sequel Pro could not find any columns belonging to this index. Maybe it has been removed already?", @"table structure : indexes : delete index : no columns error : description")
);
return;
}

[myColumns sortUsingSelector:@selector(compare:)];

//now let's find a matching fk (ie. one that has the same columns as the index)
for (NSDictionary *fkInfo in [tableData getConstraints]) {
NSArray *fkColumns = [[fkInfo objectForKey:@"columns"] sortedArrayUsingSelector:@selector(compare:)];
if(![myColumns isEqualToArray:fkColumns]) continue;
if(constraintName != nil) {
goto no_or_multiple_matches; //we already found a matching FK, but there is another one!? -> abort
}
constraintName = [fkInfo objectForKey:@"name"];
}

if(!constraintName) goto no_or_multiple_matches; //we found no matching FK

NSAlert *alert = [NSAlert alertWithMessageText:NSLocalizedString(@"A foreign key needs this index", @"table structure : indexes : delete index : error 1553 : title")
defaultButton:NSLocalizedString(@"Delete Both", @"table structure : indexes : delete index : error 1553 : delete index and FK button")
alternateButton:NSLocalizedString(@"Cancel", @"cancel button")
otherButton:nil
informativeTextWithFormat:NSLocalizedString(@"The foreign key relationship '%@' has a dependency on index '%@'. This relationship must be removed before the index can be deleted.\n\nAre you sure you want to continue to delete the relationship and the index? This action cannot be undone.", @"table structure : indexes : delete index : error 1553 : description"), constraintName, keyName];

[alert setAlertStyle:NSCriticalAlertStyle];

NSArray *buttons = [alert buttons];

// Change the alert's cancel button to have the key equivalent of return
[[buttons objectAtIndex:0] setKeyEquivalent:@"d"];
[[buttons objectAtIndex:0] setKeyEquivalentModifierMask:NSCommandKeyMask];
[[buttons objectAtIndex:1] setKeyEquivalent:@"\r"];

[alert beginSheetModalForWindow:[dbDocument parentWindow]
modalDelegate:self
didEndSelector:@selector(removeIndexSheetDidEnd:returnCode:contextInfo:)
contextInfo:[@{@"Key_name" : keyName, @"ForeignKey": constraintName} retain]]; // contextInfo is NOT retained by Cocoa!

return;

no_or_multiple_matches:
SPOnewayAlertSheet(
NSLocalizedString(@"A foreign key needs this index", @"table structure : indexes : delete index : error 1553, no FK found : title"),
[dbDocument parentWindow],
[NSString stringWithFormat:NSLocalizedString(@"This index cannot be deleted, because it is used by an existing foreign key relationship.\n\nPlease remove the relationship, before trying to remove this index.\n\nMySQL said: %@", @"table structure : indexes : delete index : error 1553, no FK found : description"), [info objectForKey:@"error"]]
);
}

/**
* Resizes the new index sheet's height by the supplied delta, while retaining the position of
* all interface controls to accommodate the advanced options view.
Expand Down

0 comments on commit e52546b

Please sign in to comment.