sql: Restore strict DCHECK of cached statements SQL.
https://crrev.com/c/1338177 relaxed the DCHECKs for
sql::Database::GetCachedStatment by allowing the SQL statement argument
to have leading and trailing whitespace when compared with the SQL
statement reported by SQLite.
This CL reverts the relaxed restriction. Instead, GetCachedStatement
will also DCHECK that the SQL statement argument matches the SQL
statement reported by SQLite when preparing the statement. This will
help developers discover non-canonical statements on first use.
Bug: 894884
Change-Id: I166f8cda8d5a3980d9858a26507f778bc87ee6cc
Reviewed-on: https://chromium-review.googlesource.com/c/1341493
Commit-Queue: Victor Costan <[email protected]>
Reviewed-by: Joshua Bell <[email protected]>
Cr-Commit-Position: refs/heads/master@{#609605}diff --git a/sql/database.h b/sql/database.h
index 7bee2567..8dc0dc4 100644
--- a/sql/database.h
+++ b/sql/database.h
@@ -376,19 +376,23 @@
// keeping commonly-used ones around for future use is important for
// performance.
//
+ // The SQL_FROM_HERE macro is the recommended way of generating a StatementID.
+ // Code that generates custom IDs must ensure that a StatementID is never used
+ // for different SQL statements. Failing to meet this requirement results in
+ // incorrect behavior, and should be caught by a DCHECK.
+ //
+ // The SQL statement passed in |sql| must match the SQL statement reported
+ // back by SQLite. Mismatches are caught by a DCHECK, so any code that has
+ // automated test coverage or that was manually tested on a DCHECK build will
+ // not exhibit this problem. Mismatches generally imply that the statement
+ // passed in has extra whitespace or comments surrounding it, which waste
+ // storage and CPU cycles.
+ //
// If the |sql| has an error, an invalid, inert StatementRef is returned (and
// the code will crash in debug). The caller must deal with this eventuality,
// either by checking validity of the |sql| before calling, by correctly
// handling the return of an inert statement, or both.
//
- // The StatementID and the SQL must always correspond to one-another. The
- // ID is the lookup into the cache, so crazy things will happen if you use
- // different SQL with the same ID.
- //
- // You will normally use the SQL_FROM_HERE macro to generate a statement
- // ID associated with the current line of code. This gives uniqueness without
- // you having to manage unique names. See StatementID above for more.
- //
// Example:
// sql::Statement stmt(database_.GetCachedStatement(
// SQL_FROM_HERE, "SELECT * FROM foo"));
