ccgus/fmdb — pull request #267 by robertmryan (2014-05-14 04:07:45+00:00)
Title: Change executeQueryWithFormat to conform to standard printf-style formatters

Removed custom `extractSQL` method, replacing it with the standard Cocoa `stringWithFormat` method.

**Note, this changes the behavior of `executeUpdateWithFormat` and `executeQueryWithFormat`.**

For example, there was a test case for the following SQL:

[self.db executeUpdateWithFormat:@"insert into t5 values (%s, %d, %@, %c, %lld)", "text", 42, @"BLOB", 'd', 12345678901234ll];

Historically, the `extractSQL` routine was rendering the above as:

[self.db executeUpdate:@"insert into t5 values (?, ?, ?, ?, ?)", @"text", @42, @"BLOB", @"d", @(12345678901234ll)];

While this was a noble effort to change a non-placeholder SQL into one that was, this is not in conformance with `stringWithFormat` with its `printf`-style formatters. It strikes me that the correct implementation of the above SQL should have been (note the single quotation marks in the SQL):'

[self.db executeUpdateWithFormat:@"insert into t5 values ('%s', %d, '%@', '%c', %lld)", "text", 42, @"BLOB", 'd', 12345678901234ll];

If someone wanted to not have to deal with the quotation problems with `executeUpdateWithFormat`, then they would have used `executeUpdate`:

[self.db executeUpdate:@"insert into t5 values (?, ?, ?, ?, ?)", @"text", @42, @"BLOB", @"d", @(12345678901234ll)];

This commit is not just a pedantic observation that “`executeFormatWithUpdate` doesn’t conform to standard `printf`-style formatting” (while that is true). The reason this commit is important, is that one might want to use `executeUpdateWithFormat` and `executeQueryWithFormat` in situations where the `?` placeholder is not permitted. Consider this example where the parameters being passed to `executeQueryWithFormat` are table names or column names.

tableName = @"customer";
fieldName = @"firstname";
rs = [self.db executeQueryWithFormat:@"SELECT COUNT(*) FROM %@ WHERE %@ = 'Ryan'", tableName, fieldName];
XCTAssert(rs, @"executeUpdateWithFormat failed: %@", [self.db lastErrorMessage]);
if ([rs next]) {
NSInteger count = [rs intForColumnIndex:0];
XCTAssertEqual(count, 0, @"We should get zero hits");
}
[rs close];

That would not work using the old `extractSQL` technique. When you use the `NSString` method `initWithFormat`, the above now works fine.

Another example where the `extractSQL` technique was problematic was when doing `PRAGMA` statements, where `?` placeholders are not permitted:

BOOL success = [self.db executeUpdateWithFormat:@"PRAGMA ENCODING ='%@'", @"UTF-8"];

XCTAssert(success, @"pragma encoding failed because of it does not support ? placeholder: %@", [self.db lastErrorMessage]);

This would fail under the old `extractSQL` technique, but work in using the `initWithString` technique.

Both of the two above examples have been added as test cases, which you can use to compare and contrast the new behavior against the old code.

I understand that this is a non-trivial commit, as this changes the behavior of `executeQueryWithFormat` and `executeUpdateWithFormat`, violating backward compatibility. But I think the old behavior is incorrect. Still, if you wanted to defer this commit, I would understand completely. But I believe that this is a more appropriate implementation of these methods (and coincidentally, simplifies the code), but I will defer to you as to whether (a) you think it’s appropriate; and (b) if and when you choose to roll it out.