summaryrefslogtreecommitdiffstats
path: root/sql/connection.cc
diff options
context:
space:
mode:
authorshess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-12 23:50:59 +0000
committershess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-12 23:50:59 +0000
commiteff1fa525ce9cf8965cf187c9e7aef8c2db39bf9 (patch)
tree11404979e9dfe31379468712d8b3acf5c552b345 /sql/connection.cc
parentd2528e607d9f876b4db237ebf37a57dcd45a4dbf (diff)
downloadchromium_src-eff1fa525ce9cf8965cf187c9e7aef8c2db39bf9.zip
chromium_src-eff1fa525ce9cf8965cf187c9e7aef8c2db39bf9.tar.gz
chromium_src-eff1fa525ce9cf8965cf187c9e7aef8c2db39bf9.tar.bz2
Put debugging assertions into sql::Statement.
Pulls out the core of gbillock's http://codereview.chromium.org/8283002/ - Move NOTREACHED and similar checks into the sql:: implementation code. - Add malformed SQL checks to Connection::Execute. - Add SQL-checking convenience methods to Connection. The general idea is that the sql:: framework assumes valid statements, rather than having client code contain scattered ad-hoc (and thus inconsistent) checks. This version puts back Statement operator overloading and loosy-goosy Execute() calls to allow other code to be updated in small batches. R=gbillock@chromium.org,jhawkins@chromium.org,dhollowa@chromium.org BUG=none TEST=sql_unittests,unit_tests:*Table*.* Review URL: http://codereview.chromium.org/8899012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114118 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sql/connection.cc')
-rw-r--r--sql/connection.cc66
1 files changed, 38 insertions, 28 deletions
diff --git a/sql/connection.cc b/sql/connection.cc
index f7fef12..26e71fd 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -132,7 +132,7 @@ void Connection::Close() {
void Connection::Preload() {
if (!db_) {
- NOTREACHED();
+ DLOG(FATAL) << "Cannot preload null db";
return;
}
@@ -142,7 +142,7 @@ void Connection::Preload() {
if (!DoesTableExist("meta"))
return;
Statement dummy(GetUniqueStatement("SELECT * FROM meta"));
- if (!dummy || !dummy.Step())
+ if (!dummy.Step())
return;
#if !defined(USE_SYSTEM_SQLITE)
@@ -166,7 +166,7 @@ bool Connection::BeginTransaction() {
needs_rollback_ = false;
Statement begin(GetCachedStatement(SQL_FROM_HERE, "BEGIN TRANSACTION"));
- if (!begin || !begin.Run())
+ if (!begin.Run())
return false;
}
transaction_nesting_++;
@@ -175,7 +175,7 @@ bool Connection::BeginTransaction() {
void Connection::RollbackTransaction() {
if (!transaction_nesting_) {
- NOTREACHED() << "Rolling back a nonexistent transaction";
+ DLOG(FATAL) << "Rolling back a nonexistent transaction";
return;
}
@@ -192,7 +192,7 @@ void Connection::RollbackTransaction() {
bool Connection::CommitTransaction() {
if (!transaction_nesting_) {
- NOTREACHED() << "Rolling back a nonexistent transaction";
+ DLOG(FATAL) << "Rolling back a nonexistent transaction";
return false;
}
transaction_nesting_--;
@@ -208,15 +208,22 @@ bool Connection::CommitTransaction() {
}
Statement commit(GetCachedStatement(SQL_FROM_HERE, "COMMIT"));
- if (!commit)
- return false;
return commit.Run();
}
-bool Connection::Execute(const char* sql) {
+int Connection::ExecuteAndReturnErrorCode(const char* sql) {
if (!db_)
return false;
- return sqlite3_exec(db_, sql, NULL, NULL, NULL) == SQLITE_OK;
+ return sqlite3_exec(db_, sql, NULL, NULL, NULL);
+}
+
+bool Connection::Execute(const char* sql) {
+ int error = ExecuteAndReturnErrorCode(sql);
+ // TODO(shess,gbillock): DLOG(FATAL) once Execute() clients are
+ // converted.
+ if (error == SQLITE_ERROR)
+ DLOG(ERROR) << "SQL Error in " << sql << ", " << GetErrorMessage();
+ return error == SQLITE_OK;
}
bool Connection::ExecuteWithTimeout(const char* sql, base::TimeDelta timeout) {
@@ -225,7 +232,7 @@ bool Connection::ExecuteWithTimeout(const char* sql, base::TimeDelta timeout) {
ScopedBusyTimeout busy_timeout(db_);
busy_timeout.SetTimeout(timeout);
- return sqlite3_exec(db_, sql, NULL, NULL, NULL) == SQLITE_OK;
+ return Execute(sql);
}
bool Connection::HasCachedStatement(const StatementID& id) const {
@@ -259,22 +266,28 @@ scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
sqlite3_stmt* stmt = NULL;
if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK) {
- // Treat this as non-fatal, it can occur in a number of valid cases, and
- // callers should be doing their own error handling.
- DLOG(WARNING) << "SQL compile error " << GetErrorMessage();
+ // This is evidence of a syntax error in the incoming SQL.
+ DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
return new StatementRef(this, NULL);
}
return new StatementRef(this, stmt);
}
+bool Connection::IsSQLValid(const char* sql) {
+ sqlite3_stmt* stmt = NULL;
+ if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK)
+ return false;
+
+ sqlite3_finalize(stmt);
+ return true;
+}
+
bool Connection::DoesTableExist(const char* table_name) const {
// GetUniqueStatement can't be const since statements may modify the
// database, but we know ours doesn't modify it, so the cast is safe.
Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
"SELECT name FROM sqlite_master "
"WHERE type='table' AND name=?"));
- if (!statement)
- return false;
statement.BindString(0, table_name);
return statement.Step(); // Table exists if any row was returned.
}
@@ -288,8 +301,6 @@ bool Connection::DoesColumnExist(const char* table_name,
// Our SQL is non-mutating, so this cast is OK.
Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
sql.c_str()));
- if (!statement)
- return false;
while (statement.Step()) {
if (!statement.ColumnString(1).compare(column_name))
@@ -300,7 +311,7 @@ bool Connection::DoesColumnExist(const char* table_name,
int64 Connection::GetLastInsertRowId() const {
if (!db_) {
- NOTREACHED();
+ DLOG(FATAL) << "Illegal use of connection without a db";
return 0;
}
return sqlite3_last_insert_rowid(db_);
@@ -308,7 +319,7 @@ int64 Connection::GetLastInsertRowId() const {
int Connection::GetLastChangeCount() const {
if (!db_) {
- NOTREACHED();
+ DLOG(FATAL) << "Illegal use of connection without a db";
return 0;
}
return sqlite3_changes(db_);
@@ -339,7 +350,7 @@ const char* Connection::GetErrorMessage() const {
bool Connection::OpenInternal(const std::string& file_name) {
if (db_) {
- NOTREACHED() << "sql::Connection is already open.";
+ DLOG(FATAL) << "sql::Connection is already open.";
return false;
}
@@ -370,7 +381,7 @@ bool Connection::OpenInternal(const std::string& file_name) {
// which requests exclusive locking but doesn't get it is almost
// certain to be ill-tested.
if (!Execute("PRAGMA locking_mode=EXCLUSIVE"))
- NOTREACHED() << "Could not set locking mode: " << GetErrorMessage();
+ DLOG(FATAL) << "Could not set locking mode: " << GetErrorMessage();
}
const base::TimeDelta kBusyTimeout =
@@ -384,17 +395,17 @@ bool Connection::OpenInternal(const std::string& file_name) {
DCHECK_LE(page_size_, kSqliteMaxPageSize);
const std::string sql = StringPrintf("PRAGMA page_size=%d", page_size_);
if (!ExecuteWithTimeout(sql.c_str(), kBusyTimeout))
- NOTREACHED() << "Could not set page size: " << GetErrorMessage();
+ DLOG(FATAL) << "Could not set page size: " << GetErrorMessage();
}
if (cache_size_ != 0) {
const std::string sql = StringPrintf("PRAGMA cache_size=%d", cache_size_);
if (!ExecuteWithTimeout(sql.c_str(), kBusyTimeout))
- NOTREACHED() << "Could not set cache size: " << GetErrorMessage();
+ DLOG(FATAL) << "Could not set cache size: " << GetErrorMessage();
}
if (!ExecuteWithTimeout("PRAGMA secure_delete=ON", kBusyTimeout)) {
- NOTREACHED() << "Could not enable secure_delete: " << GetErrorMessage();
+ DLOG(FATAL) << "Could not enable secure_delete: " << GetErrorMessage();
Close();
return false;
}
@@ -404,8 +415,7 @@ bool Connection::OpenInternal(const std::string& file_name) {
void Connection::DoRollback() {
Statement rollback(GetCachedStatement(SQL_FROM_HERE, "ROLLBACK"));
- if (rollback)
- rollback.Run();
+ rollback.Run();
}
void Connection::StatementRefCreated(StatementRef* ref) {
@@ -416,7 +426,7 @@ void Connection::StatementRefCreated(StatementRef* ref) {
void Connection::StatementRefDeleted(StatementRef* ref) {
StatementRefSet::iterator i = open_statements_.find(ref);
if (i == open_statements_.end())
- NOTREACHED();
+ DLOG(FATAL) << "Could not find statement";
else
open_statements_.erase(i);
}
@@ -436,7 +446,7 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt) {
if (error_delegate_.get())
return error_delegate_->OnError(err, this, stmt);
// The default handling is to assert on debug and to ignore on release.
- NOTREACHED() << GetErrorMessage();
+ DLOG(FATAL) << GetErrorMessage();
return err;
}
generated by cgit v1.1 at 2025-03-03 12:24:49 +0000