Improve the way Sequel Pro inferrs the collation of a column. (#2237)

This does not entirely fix the bug of SP sometimes displaying the wrong collation, but should work in >= 99% of cases.

4 files changed, 91 insertions, 28 deletions

diff --git a/Source/SPDatabaseData.h b/Source/SPDatabaseData.h
index aaf92f3d..f2fe08e1 100644
--- a/Source/SPDatabaseData.h
+++ b/Source/SPDatabaseData.h
@@ -42,6 +42,7 @@
 @interface SPDatabaseData : NSObject 
 {
 	NSString *characterSetEncoding;
+	NSString *defaultCollationForCharacterSet;
 	NSString *defaultCharacterSetEncoding;
 	NSString *defaultCollation;
 	NSString *serverDefaultCharacterSetEncoding;
@@ -72,6 +73,7 @@
 
 - (NSArray *)getDatabaseCollations;
 - (NSArray *)getDatabaseCollationsForEncoding:(NSString *)encoding;
+- (NSString *)getDefaultCollationForEncoding:(NSString *)encoding;
 - (NSArray *)getDatabaseStorageEngines;
 - (NSArray *)getDatabaseCharacterSetEncodings;
 
diff --git a/Source/SPDatabaseData.m b/Source/SPDatabaseData.m
index 4856505d..2d1d4f14 100644
--- a/Source/SPDatabaseData.m
+++ b/Source/SPDatabaseData.m
@@ -58,6 +58,7 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 {
 	if ((self = [super init])) {
 		characterSetEncoding = nil;
+		defaultCollationForCharacterSet = nil;
 		defaultCollation = nil;
 		defaultCharacterSetEncoding = nil;
 		serverDefaultCollation = nil;
@@ -83,6 +84,7 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 - (void)resetAllData
 {
 	if (characterSetEncoding != nil) SPClear(characterSetEncoding);
+	if (defaultCollationForCharacterSet != nil) SPClear(defaultCollationForCharacterSet);
 	if (defaultCollation != nil) SPClear(defaultCollation);
 	if (defaultCharacterSetEncoding != nil) SPClear(defaultCharacterSetEncoding);
 	if (serverDefaultCharacterSetEncoding) SPClear(serverDefaultCharacterSetEncoding);
@@ -117,9 +119,14 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 		// If that failed, get the list of collations from the hard-coded list
 		if (![collations count]) {
 			const SPDatabaseCharSets *c = SPGetDatabaseCharacterSets();
-			
+#warning This probably won't work as intended. See my comment in getDatabaseCollationsForEncoding:
 			do {
-				[collations addObject:[NSString stringWithCString:c->collation encoding:NSUTF8StringEncoding]];
+				[collations addObject:@{
+					@"ID"                 : @(c->nr),
+					@"CHARACTER_SET_NAME" : [NSString stringWithCString:c->name encoding:NSUTF8StringEncoding],
+					@"COLLATION_NAME"     : [NSString stringWithCString:c->collation encoding:NSUTF8StringEncoding],
+					// description is not present in information_schema.collations
+				}];
 				
 				++c;
 			} 
@@ -139,12 +146,16 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 	if (encoding && ((characterSetEncoding == nil) || (![characterSetEncoding isEqualToString:encoding]) || ([characterSetCollations count] == 0))) {
 		
 		[characterSetEncoding release];
+		SPClear(defaultCollationForCharacterSet); //depends on encoding
 		[characterSetCollations removeAllObjects];
 		
 		characterSetEncoding = [[NSString alloc] initWithString:encoding];
 
-		if([cachedCollationsByEncoding objectForKey:characterSetEncoding] && [[cachedCollationsByEncoding objectForKey:characterSetEncoding] count])
-			return [cachedCollationsByEncoding objectForKey:characterSetEncoding];
+		NSArray *cachedCollations = [cachedCollationsByEncoding objectForKey:characterSetEncoding];
+		if([cachedCollations count]) {
+			[characterSetCollations addObjectsFromArray:cachedCollations];
+			goto copy_return;
+		}
 
 		// Try to retrieve the available collations for the supplied encoding from the database
 		if ([serverSupport supportsInformationSchema]) {
@@ -162,7 +173,9 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 		// If that failed, get the list of collations matching the supplied encoding from the hard-coded list
 		if (![characterSetCollations count]) {
 			const SPDatabaseCharSets *c = SPGetDatabaseCharacterSets();
-			
+#warning I don't think this will work. The hardcoded list is supposed to be used with pre 4.1 mysql servers, \
+         which don't have information_schema or SHOW COLLATION. But before 4.1 there were no real collations and \
+         even the charsets had different names (e.g. charset "latin1_de" which now is "latin1" + "latin1_german2_ci")
 			do {
 				NSString *charSet = [NSString stringWithCString:c->name encoding:NSUTF8StringEncoding];
 
@@ -175,13 +188,38 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 			while (c[0].nr != 0);
 		}
 
-		if (characterSetCollations && [characterSetCollations count]) {
+		if ([characterSetCollations count]) {
 			[cachedCollationsByEncoding setObject:[NSArray arrayWithArray:characterSetCollations] forKey:characterSetEncoding];
 		}
 
 	}
-	
-	return characterSetCollations;
+copy_return:
+	return [NSArray arrayWithArray:characterSetCollations]; //copy because it is a mutable array and we keep changing it
+}
+
+/** Get the collation that is marked as default for a given encoding by the server
+ * @param encoding The encoding, e.g. @"latin1"
+ * @return The default collation (e.g. @"latin1_swedish_ci") or 
+ *         nil if either encoding was nil or the server does not provide the neccesary details
+ */
+- (NSString *)getDefaultCollationForEncoding:(NSString *)encoding
+{
+	if(!encoding) return nil;
+	// if (
+	//   - we have not yet fetched info about the default collation OR
+	//   - encoding is different than the one we currently know about
+	// ) => we need to load it from server, otherwise just return cached value
+	if ((defaultCollationForCharacterSet == nil) || (![characterSetEncoding isEqualToString:encoding])) {
+		NSArray *cols = [self getDatabaseCollationsForEncoding:encoding]; //will clear stored encoding and collation if neccesary
+		for (NSDictionary *collation in cols) {
+#warning This won't work for the hardcoded list (see above)
+			if([[[collation objectForKey:@"IS_DEFAULT"] lowercaseString] isEqualToString:@"yes"]) {
+				defaultCollationForCharacterSet = [[NSString alloc] initWithString:[collation objectForKey:@"COLLATION_NAME"]];
+				break;
+			}
+		}
+	}
+	return defaultCollationForCharacterSet;
 }
 
 /**
@@ -296,7 +334,7 @@ NSInteger _sortStorageEngineEntry(NSDictionary *itemOne, NSDictionary *itemTwo,
 		// If that failed, get the list of character set encodings from the hard-coded list
 		if (![characterSetEncodings count]) {			
 			const SPDatabaseCharSets *c = SPGetDatabaseCharacterSets();
-
+#warning This probably won't work as intended. See my comment in getDatabaseCollationsForEncoding:
 			do {
 				[characterSetEncodings addObject:[NSDictionary dictionaryWithObjectsAndKeys:
 					[NSString stringWithCString:c->name encoding:NSUTF8StringEncoding], @"CHARACTER_SET_NAME",
diff --git a/Source/SPTableStructureDelegate.m b/Source/SPTableStructureDelegate.m
index c027d339..b901e858 100644
--- a/Source/SPTableStructureDelegate.m
+++ b/Source/SPTableStructureDelegate.m
@@ -62,6 +62,8 @@
 	if ((NSUInteger)rowIndex >= [tableFields count]) return @"...";
 	
 	if ([[tableColumn identifier] isEqualToString:@"collation"]) {
+		NSString *tableEncoding = [tableDataInstance tableEncoding];
+		NSString *columnEncoding = nil;
 		NSInteger idx = 0;
 		
 		if ((idx = [[NSArrayObjectAtIndex(tableFields, rowIndex) objectForKey:@"encoding"] integerValue]) > 0 && idx < [encodingPopupCell numberOfItems]) {
@@ -69,28 +71,44 @@
 
 			NSUInteger start = [enc rangeOfString:@"("].location + 1;
 			
-			collations = [databaseDataInstance getDatabaseCollationsForEncoding:[enc substringWithRange:NSMakeRange(start, [enc length] - start - 1)]];
+			columnEncoding = [enc substringWithRange:NSMakeRange(start, [enc length] - start - 1)];
+			collations = [databaseDataInstance getDatabaseCollationsForEncoding:columnEncoding];
 		} 
 		else {
 			// If the structure has loaded (not still loading!) and the table encoding
 			// is set, use the appropriate collations.
-			collations = ([tableDocumentInstance structureLoaded] && [tableDataInstance tableEncoding] != nil) ? [databaseDataInstance getDatabaseCollationsForEncoding:[tableDataInstance tableEncoding]] : @[];
+			collations = @[];
+			if([tableDocumentInstance structureLoaded]) {
+				columnEncoding = [tableDataInstance tableEncoding];
+				if(columnEncoding) collations = [databaseDataInstance getDatabaseCollationsForEncoding:columnEncoding];
+			}
 		}
 		
-		[[tableColumn dataCell] removeAllItems];
+		NSPopUpButtonCell *collationCell = [tableColumn dataCell];
+		
+		[collationCell removeAllItems];
 		
 		if ([collations count] > 0) {
-			NSString *defaultCollation = [[tableDataInstance statusValues] objectForKey:@"collation"];
+			NSString *tableCollation = [[tableDataInstance statusValues] objectForKey:@"Collation"];
 			
-			if (!defaultCollation) {
-				defaultCollation = [databaseDataInstance getDatabaseDefaultCollation];
+			if (![tableCollation length]) {
+				tableCollation = [databaseDataInstance getDefaultCollationForEncoding:tableEncoding];
+			}
+			
+			NSString *columnCollation = [NSArrayObjectAtIndex(tableFields, rowIndex) objectForKey:@"collationName"];
+			
+			if (![columnCollation length]) {
+				columnCollation = [databaseDataInstance getDefaultCollationForEncoding:columnEncoding];
 			}
 			
 			[[tableColumn dataCell] addItemWithTitle:@""];
 
 			BOOL useMonospacedFont = [prefs boolForKey:SPUseMonospacedFonts];
 			CGFloat monospacedFontSize = [prefs floatForKey:SPMonospacedFontSize] > 0 ? [prefs floatForKey:SPMonospacedFontSize] : [NSFont smallSystemFontSize];
+			NSMutableDictionary *menuAttributes = [NSMutableDictionary dictionaryWithObject:[NSColor lightGrayColor] forKey:NSForegroundColorAttributeName];
+			[menuAttributes setObject:useMonospacedFont ? [NSFont fontWithName:SPDefaultMonospacedFontName size:monospacedFontSize] : [NSFont systemFontOfSize:[NSFont smallSystemFontSize]] forKey:NSFontAttributeName];
 			
+			BOOL columnUsesTableDefaultEncoding = ([columnEncoding isEqualToString:tableEncoding]);
 			// Populate collation popup button
 			for (NSDictionary *collation in collations)
 			{
@@ -99,12 +117,9 @@
 				[[tableColumn dataCell] addItemWithTitle:collationName];
 
 				// If this matches the table's collation, draw in gray
-				if ([collationName length] && [collationName isEqualToString:defaultCollation]) {
+				if (columnUsesTableDefaultEncoding && [collationName isEqualToString:tableCollation]) {
 					NSMenuItem *collationMenuItem = [(NSPopUpButtonCell *)[tableColumn dataCell] itemAtIndex:([[tableColumn dataCell] numberOfItems] - 1)];
-					NSMutableDictionary *menuAttributes = [NSMutableDictionary dictionaryWithObject:[NSColor lightGrayColor] forKey:NSForegroundColorAttributeName];
-										
-					[menuAttributes setObject:useMonospacedFont ? [NSFont fontWithName:SPDefaultMonospacedFontName size:monospacedFontSize] : [NSFont systemFontOfSize:[NSFont smallSystemFontSize]] forKey:NSFontAttributeName];
-					
+
 					NSAttributedString *itemString = [[[NSAttributedString alloc] initWithString:collationName attributes:menuAttributes] autorelease];
 					
 					[collationMenuItem setAttributedTitle:itemString];
diff --git a/Source/SPTableStructureLoading.m b/Source/SPTableStructureLoading.m
index 1950c32e..05004932 100644
--- a/Source/SPTableStructureLoading.m
+++ b/Source/SPTableStructureLoading.m
@@ -172,14 +172,20 @@
 		NSString *encoding = nil;
 		
 		if ([fieldValidation isFieldTypeString:type]) {
-						
-			collation = [theField objectForKey:@"collation"] ? [theField objectForKey:@"collation"] : [[tableDataInstance statusValues] objectForKey:@"collation"];
-			encoding = [theField objectForKey:@"encoding"] ? [theField objectForKey:@"encoding"] : [tableDataInstance tableEncoding];
 			
-			// If we still don't have a collation then fallback on the database default (not available on MySQL < 4.1.1).
-			if (!collation) {
-				collation = [databaseDataInstance getDatabaseDefaultCollation];
-			}
+			// The MySQL 4.1 manual says:
+			//
+			// MySQL chooses the column character set and collation in the following manner:
+			//   1. If both CHARACTER SET X and COLLATE Y were specified, then character set X and collation Y are used.
+			//   2. If CHARACTER SET X was specified without COLLATE, then character set X and its default collation are used.
+			//   3. If COLLATE Y was specified without CHARACTER SET, then the character set associated with Y and collation Y.
+			//   4. Otherwise, the table character set and collation are used.
+#warning This is not correct, see the comment above. \
+         However MySQL ususally outputs the CREATE TABLE statement in a way for this to still get the result right.
+			encoding  = [theField objectForKey:@"encoding"]  ? [theField objectForKey:@"encoding"]  : [tableDataInstance tableEncoding];
+			collation = [theField objectForKey:@"collation"] ? [theField objectForKey:@"collation"] : [databaseDataInstance getDefaultCollationForEncoding:encoding];
+			
+			// MySQL < 4.1 does not support collations (they are part of the charset), it will be nil there
 		}
 
 		if (encoding) {
@@ -189,7 +195,8 @@
 					fieldEncoding = encoding;
 
 					// Set the selected index as the match index +1 due to the leading @"" in the popup list
-					[theField setObject:[NSNumber numberWithInteger:(selectedIndex + 1)] forKey:@"encoding"];
+					[theField setObject:@(selectedIndex + 1) forKey:@"encoding"];
+					[theField setObject:encoding forKey:@"encodingName"];
 					break;
 				}
 				
@@ -209,6 +216,7 @@
 					
 					// Set the selected index as the match index +1 due to the leading @"" in the popup list
 					[theField setObject:[NSNumber numberWithInteger:(selectedIndex + 1)] forKey:@"collation"];
+					[theField setObject:collation forKey:@"collationName"];
 
 					// Set BINARY if collation ends with _bin for convenience
 					if ([[col objectForKey:@"COLLATION_NAME"] hasSuffix:@"_bin"]) {