sql: Make Database::ExecuteAndReturnErrorCode() private.
The Database::Execute() guidance currently points to
ExecuteAndReturnErrorCode() for situations where errors would be
ignored. However, the guidance is ignored [1, 2], and
ExecuteAndReturnErrorCode() is not used outside of //sql.
This CL makes ExecuteAndReturnErrorCode() private to Database, and
updates testing code that relied on it. This is an architectural
improvement, as code outside //sql should not have to know about SQLite
error codes.
[1] https://source.chromium.org/search?q=ignore_result%5C(.*%5C.Execute%5C(
[2] https://source.chromium.org/search?q=ignore_result%5C(.*-%3EExecute%5C(
Change-Id: I8d58180a40094c50e1da12b1445fb94c88531a35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043151
Auto-Submit: Victor Costan <[email protected]>
Commit-Queue: Marijn Kruisselbrink <[email protected]>
Reviewed-by: Marijn Kruisselbrink <[email protected]>
Cr-Commit-Position: refs/heads/master@{#904027}
diff --git a/sql/database.h b/sql/database.h
index ca81a62e..92ec6cf 100644
--- a/sql/database.h
+++ b/sql/database.h
@@ -395,19 +395,18 @@
// Statements ----------------------------------------------------------------
- // Executes the given SQL string, returning true on success. This is
- // normally used for simple, 1-off statements that don't take any bound
- // parameters and don't return any data (e.g. CREATE TABLE).
+ // Executes a SQL statement. Returns true for success, and false for failure.
//
- // This will DCHECK if the |sql| contains errors.
+ // `sql` should be a single SQL statement. Production code should not execute
+ // multiple SQL statements at once, to facilitate crash debugging.
//
- // Do not use ignore_result() to ignore all errors. Use
- // ExecuteAndReturnErrorCode() and ignore only specific errors.
+ // TODO(crbug.com/1230443): Migrate testing code that executes multiple SQL
+ // statements to a separate method.
+ //
+ // `sql` cannot have parameters. Statements with parameters can be handled by
+ // sql::Statement. See GetCachedStatement() and GetUniqueStatement().
bool Execute(const char* sql) WARN_UNUSED_RESULT;
- // Like Execute(), but returns the error code given by SQLite.
- int ExecuteAndReturnErrorCode(const char* sql) WARN_UNUSED_RESULT;
-
// Returns a statement for the given SQL using the statement cache. It can
// take a nontrivial amount of work to parse and compile a statement, so
// keeping commonly-used ones around for future use is important for
@@ -695,6 +694,12 @@
// this did not work out.
int OnSqliteError(int err, Statement* stmt, const char* sql) const;
+ // Like Execute(), but returns the error code given by SQLite.
+ //
+ // This is only exposed to the Database implementation. Code that uses
+ // sql::Database should not be concerned with SQLite error codes.
+ int ExecuteAndReturnErrorCode(const char* sql) WARN_UNUSED_RESULT;
+
// Like |Execute()|, but retries if the database is locked.
bool ExecuteWithTimeout(const char* sql,
base::TimeDelta ms_timeout) WARN_UNUSED_RESULT;
