sql: Use SQLite API instead of PRAGMA to check for column existence.
sql::Database::DoesColumnExist currently compiles and runs a "PRAGMA table_info" [1] SQL query and iterates over the result looking for the desired column name. This is more brittle and expensive than using the the sqlite3_table_column_metadata() [2] API, which pulls schema data directly, bypassing query compilation and execution. This CL introduces a behavior change -- the previous PRAGMA table_info implementation works for views [3, 4], whereas sqlite3_table_column_metadata() only works for tables. The behavior change is not visible, because Chrome currently uses views in a single feature [5], namely extensions [6]. DoesColumnExist should only be used for migrations, which should operate on concrete tables. In steady-state, each feature should have the database schema migrated to the current version, so column existence is known statically. So, removing support for views in DoesColumnExist() is acceptable. [1] https://www.sqlite.org/pragma.html#pragma_table_info [2] https://www.sqlite.org/c3ref/table_column_metadata.html [3] https://www.sqlite.org/matrix/pragma.html [4] https://www.sqlite.org/matrix/matrix_dpragma.html#R-35224-32827-55933-44478-17553-06933-55583-57822 [5] https://cs.chromium.org/search/?q=file:%5C.cc+sql::+VIEW+case:yes [6] https://cs.chromium.org/chromium/src/chrome/browser/extensions/activity_log/counting_policy.cc?q=VIEW+case:yes Bug: 910955 Change-Id: I44474585e6bb2523b5cd91c76c3444058822e601 Reviewed-on: https://chromium-review.googlesource.com/c/1357825 Reviewed-by: Chris Mumford <cmumford@google.com> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#614663}
This commit is contained in:

committed by
Commit Bot

parent
7f306df5fe
commit
1ff47e97f7
@@ -1490,23 +1490,15 @@ bool Database::DoesSchemaItemExist(const char* name, const char* type) const {
|
||||
|
||||
bool Database::DoesColumnExist(const char* table_name,
|
||||
const char* column_name) const {
|
||||
std::string sql("PRAGMA TABLE_INFO(");
|
||||
sql.append(table_name);
|
||||
sql.append(")");
|
||||
|
||||
Statement statement(GetUntrackedStatement(sql.c_str()));
|
||||
|
||||
// This can happen if the database is corrupt and the error is a test
|
||||
// expectation.
|
||||
if (!statement.is_valid())
|
||||
return false;
|
||||
|
||||
while (statement.Step()) {
|
||||
if (base::EqualsCaseInsensitiveASCII(statement.ColumnString(1),
|
||||
column_name))
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
// sqlite3_table_column_metadata uses out-params to return column definition
|
||||
// details, such as the column type and whether it allows NULL values. These
|
||||
// aren't needed to compute the current method's result, so we pass in nullptr
|
||||
// for all the out-params.
|
||||
int error = sqlite3_table_column_metadata(
|
||||
db_, "main", table_name, column_name, /* pzDataType= */ nullptr,
|
||||
/* pzCollSeq= */ nullptr, /* pNotNull= */ nullptr,
|
||||
/* pPrimaryKey= */ nullptr, /* pAutoinc= */ nullptr);
|
||||
return error == SQLITE_OK;
|
||||
}
|
||||
|
||||
int64_t Database::GetLastInsertRowId() const {
|
||||
|
@@ -420,6 +420,12 @@ class SQL_EXPORT Database {
|
||||
bool DoesViewExist(const char* table_name) const;
|
||||
|
||||
// Returns true if a column with the given name exists in the given table.
|
||||
//
|
||||
// Calling this method on a VIEW returns an unspecified result.
|
||||
//
|
||||
// This should only be used by migration code for legacy features that do not
|
||||
// use MetaTable, and need an alternative way of figuring out the database's
|
||||
// current version.
|
||||
bool DoesColumnExist(const char* table_name, const char* column_name) const;
|
||||
|
||||
// Returns sqlite's internal ID for the last inserted row. Valid only
|
||||
|
Reference in New Issue
Block a user