Просмотр исходного кода

Fixed a problem where having statement caching turned on would cause issues when trying to use two result sets with the same query but different binding parameters. Thanks to Nick Hodapp for the original patch.

August Mueller 12 лет назад
Родитель
Сommit
cac897ed87
6 измененных файлов с 78 добавлено и 10 удалено
  1. 3 0
      CHANGES_AND_TODO_LIST.txt
  2. 1 0
      CONTRIBUTORS.txt
  3. 2 0
      src/FMDatabase.h
  4. 25 6
      src/FMDatabase.m
  5. 3 0
      src/FMResultSet.m
  6. 44 4
      src/fmdb.m

+ 3 - 0
CHANGES_AND_TODO_LIST.txt

@@ -3,6 +3,9 @@ Zip, nada, zilch.  Got any ideas?
 
 If you would like to contribute some code- awesome!  I just ask that you make it conform to the coding conventions already set in here, and to add a couple of tests for your new code to fmdb.m.  And of course, the code should be of general use to more than just a couple of folks.  Send your patches to gus@flyingmeat.com.
 
+2013.10.21
+    Fixed a problem where having statement caching turned on would cause issues when trying to use two result sets with the same query but different binding parameters.  Thanks to Nick Hodapp for the original patch.
+
 2013.10.16
     Added methods that expose va_list arguments.  Thanks to Ibrahim Ennafaa for the patch.
     

+ 1 - 0
CONTRIBUTORS.txt

@@ -40,5 +40,6 @@ Jim Correia
 Phillip Kast
 Chris Wright
 Joshua Tessier
+Nick Hodapp
 
 Aaaaannnd, Gus Mueller (that's me!)

+ 2 - 0
src/FMDatabase.h

@@ -943,6 +943,7 @@
     sqlite3_stmt *_statement;
     NSString *_query;
     long _useCount;
+    BOOL _inUse;
 }
 
 ///-----------------
@@ -964,6 +965,7 @@
 
 @property (atomic, assign) sqlite3_stmt *statement;
 
+@property (atomic, assign) BOOL inUse;
 
 ///----------------------------
 /// @name Closing and Resetting

+ 25 - 6
src/FMDatabase.m

@@ -163,8 +163,8 @@ - (BOOL)close {
 
 - (void)clearCachedStatements {
     
-    for (FMStatement *cachedStmt in [_cachedStatements objectEnumerator]) {
-        [cachedStmt close];
+    for (NSMutableSet *statements in [_cachedStatements objectEnumerator]) {
+        [statements makeObjectsPerformSelector:@selector(close)];
     }
     
     [_cachedStatements removeAllObjects];
@@ -195,21 +195,35 @@ - (void)resultSetDidClose:(FMResultSet *)resultSet {
 }
 
 - (FMStatement*)cachedStatementForQuery:(NSString*)query {
-    return [_cachedStatements objectForKey:query];
+    
+    NSMutableSet* statements = [_cachedStatements objectForKey:query];
+    
+    return [[statements objectsPassingTest:^BOOL(FMStatement* statement, BOOL *stop) {
+        
+        *stop = ![statement inUse];
+        return *stop;
+        
+    }] anyObject];
 }
 
+
 - (void)setCachedStatement:(FMStatement*)statement forQuery:(NSString*)query {
     
     query = [query copy]; // in case we got handed in a mutable string...
-    
     [statement setQuery:query];
     
-    [_cachedStatements setObject:statement forKey:query];
+    NSMutableSet* statements = [_cachedStatements objectForKey:query];
+    if (!statements) {
+        statements = [NSMutableSet set];
+    }
+    
+    [statements addObject:statement];
+    
+    [_cachedStatements setObject:statements forKey:query];
     
     FMDBRelease(query);
 }
 
-
 - (BOOL)rekey:(NSString*)key {
     NSData *keyData = [NSData dataWithBytes:(void *)[key UTF8String] length:(NSUInteger)strlen([key UTF8String])];
     
@@ -1212,6 +1226,7 @@ @implementation FMStatement
 @synthesize statement=_statement;
 @synthesize query=_query;
 @synthesize useCount=_useCount;
+@synthesize inUse=_inUse;
 
 - (void)finalize {
     [self close];
@@ -1231,12 +1246,16 @@ - (void)close {
         sqlite3_finalize(_statement);
         _statement = 0x00;
     }
+    
+    _inUse = NO;
 }
 
 - (void)reset {
     if (_statement) {
         sqlite3_reset(_statement);
     }
+    
+    _inUse = NO;
 }
 
 - (NSString*)description {

+ 3 - 0
src/FMResultSet.m

@@ -18,6 +18,9 @@ + (instancetype)resultSetWithStatement:(FMStatement *)statement usingParentDatab
     [rs setStatement:statement];
     [rs setParentDB:aDB];
     
+    NSParameterAssert(![statement inUse]);
+    [statement setInUse:YES]; // weak reference
+    
     return FMDBReturnAutoreleased(rs);
 }
 

+ 44 - 4
src/fmdb.m

@@ -9,6 +9,7 @@
 void testPool(NSString *dbPath);
 void testDateFormat();
 void FMDBReportABugFunction();
+void testStatementCaching();
 
 int main (int argc, const char * argv[]) {
     
@@ -1067,10 +1068,7 @@ int main (int argc, const char * argv[]) {
         }
         FMDBQuickCheck(rowCount == 2);
         
-        
-        
-        
-        
+        testStatementCaching();
         
     }];
     
@@ -1083,6 +1081,48 @@ int main (int argc, const char * argv[]) {
     
     return 0;
 }
+/*
+ Test statement caching
+ This test checks the fixes that address https://github.com/ccgus/fmdb/issues/6
+ */
+
+void testStatementCaching() {
+    
+    FMDatabase *db = [FMDatabase databaseWithPath:nil]; // use in-memory DB
+    [db open];
+    
+    [db executeUpdate:@"DROP TABLE IF EXISTS testStatementCaching"];
+    [db executeUpdate:@"CREATE TABLE testStatementCaching ( value INTEGER )"];
+    [db executeUpdate:@"INSERT INTO testStatementCaching( value ) VALUES (1)"];
+    [db executeUpdate:@"INSERT INTO testStatementCaching( value ) VALUES (1)"];
+    [db executeUpdate:@"INSERT INTO testStatementCaching( value ) VALUES (2)"];
+    
+    [db setShouldCacheStatements:YES];
+    
+    // two iterations.
+    //  the first time through no statements will be from the cache.
+    //  the second time through all statements come from the cache.
+    for (int i = 1; i <= 2; i++ ) {
+        
+        FMResultSet* rs1 = [db executeQuery: @"SELECT rowid, * FROM testStatementCaching WHERE value = ?", @1]; // results in 2 rows...
+        FMDBQuickCheck([rs1 next]);
+        
+        // confirm that we're seeing the benefits of caching.
+        FMDBQuickCheck([[rs1 statement] useCount] == i);
+        
+        FMResultSet* rs2 = [db executeQuery:@"SELECT rowid, * FROM testStatementCaching WHERE value = ?", @2]; // results in 1 row
+        FMDBQuickCheck([rs2 next]);
+        FMDBQuickCheck([[rs2 statement] useCount] == i);
+        
+        // This is the primary check - with the old implementation of statement caching, rs2 would have rejiggered the (cached) statement used by rs1, making this test fail to return the 2nd row in rs1.
+        FMDBQuickCheck([rs1 next]);
+        
+        [rs1 close];
+        [rs2 close];
+    }
+    
+    [db close];
+}
 
 /*
  Test the various FMDatabasePool things.