Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to using sqlite3_errmsg to extract a contextual error for SQLite failures #2352

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ enr
enums
EQU
ERANGE
errmsg
errno
ESRB
etest
Expand Down
12 changes: 12 additions & 0 deletions src/AppInstallerCLITests/SQLiteWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,18 @@ TEST_CASE("SQLiteWrapper_BindWithEmbeddedNull", "[sqlitewrapper]")
REQUIRE_THROWS_HR(InsertIntoSimpleTestTable(connection, firstVal, secondVal), APPINSTALLER_CLI_ERROR_BIND_WITH_EMBEDDED_NULL);
}

TEST_CASE("SQLiteWrapper_PrepareFailure", "[sqlitewrapper]")
{
Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create);

CreateSimpleTestTable(connection);

Builder::StatementBuilder builder;
builder.Select({ s_firstColumn, s_secondColumn }).From(std::string{ s_tableName } + "2").Where(s_firstColumn).Equals(2);

REQUIRE_THROWS_HR(builder.Prepare(connection), MAKE_HRESULT(SEVERITY_ERROR, FACILITY_SQLITE, SQLITE_ERROR));
}

TEST_CASE("SQLBuilder_SimpleSelectBind", "[sqlbuilder]")
{
Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create);
Expand Down
33 changes: 17 additions & 16 deletions src/AppInstallerRepositoryCore/SQLiteWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@ using namespace std::string_view_literals;
#include <stack>
#endif

#define THROW_SQLITE(_error_) \
#define THROW_SQLITE(_error_,_connection_) \
do { \
int _ts_sqliteReturnValue = _error_; \
THROW_EXCEPTION_MSG(SQLiteException(_ts_sqliteReturnValue), sqlite3_errstr(_ts_sqliteReturnValue)); \
sqlite3* _ts_sqliteConnection = _connection_; \
THROW_EXCEPTION_MSG(SQLiteException(_ts_sqliteReturnValue), _ts_sqliteConnection ? sqlite3_errmsg(_ts_sqliteConnection) : sqlite3_errstr(_ts_sqliteReturnValue)); \
} while (0,0)

#define THROW_IF_SQLITE_FAILED(_statement_) \
#define THROW_IF_SQLITE_FAILED(_statement_,_connection_) \
do { \
int _tisf_sqliteReturnValue = _statement_; \
if (_tisf_sqliteReturnValue != SQLITE_OK) \
{ \
THROW_SQLITE(_tisf_sqliteReturnValue); \
THROW_SQLITE(_tisf_sqliteReturnValue,_connection_); \
} \
} while (0,0)

Expand All @@ -47,7 +48,7 @@ namespace AppInstaller::Repository::SQLite
{
void ParameterSpecificsImpl<nullptr_t>::Bind(sqlite3_stmt* stmt, int index, nullptr_t)
{
THROW_IF_SQLITE_FAILED(sqlite3_bind_null(stmt, index));
THROW_IF_SQLITE_FAILED(sqlite3_bind_null(stmt, index), sqlite3_db_handle(stmt));
}

void ThrowIfContainsEmbeddedNullCharacter(std::string_view v)
Expand All @@ -58,7 +59,7 @@ namespace AppInstaller::Repository::SQLite
void ParameterSpecificsImpl<std::string>::Bind(sqlite3_stmt* stmt, int index, const std::string& v)
{
ThrowIfContainsEmbeddedNullCharacter(v);
THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.c_str(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8));
THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.c_str(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8), sqlite3_db_handle(stmt));
}

std::string ParameterSpecificsImpl<std::string>::GetColumn(sqlite3_stmt* stmt, int column)
Expand All @@ -77,13 +78,13 @@ namespace AppInstaller::Repository::SQLite
else
{
ThrowIfContainsEmbeddedNullCharacter(v);
THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8));
THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8), sqlite3_db_handle(stmt));
}
}

void ParameterSpecificsImpl<int>::Bind(sqlite3_stmt* stmt, int index, int v)
{
THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, v));
THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, v), sqlite3_db_handle(stmt));
}

int ParameterSpecificsImpl<int>::GetColumn(sqlite3_stmt* stmt, int column)
Expand All @@ -93,7 +94,7 @@ namespace AppInstaller::Repository::SQLite

void ParameterSpecificsImpl<int64_t>::Bind(sqlite3_stmt* stmt, int index, int64_t v)
{
THROW_IF_SQLITE_FAILED(sqlite3_bind_int64(stmt, index, v));
THROW_IF_SQLITE_FAILED(sqlite3_bind_int64(stmt, index, v), sqlite3_db_handle(stmt));
}

int64_t ParameterSpecificsImpl<int64_t>::GetColumn(sqlite3_stmt* stmt, int column)
Expand All @@ -103,7 +104,7 @@ namespace AppInstaller::Repository::SQLite

void ParameterSpecificsImpl<bool>::Bind(sqlite3_stmt* stmt, int index, bool v)
{
THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, (v ? 1 : 0)));
THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, (v ? 1 : 0)), sqlite3_db_handle(stmt));
}

bool ParameterSpecificsImpl<bool>::GetColumn(sqlite3_stmt* stmt, int column)
Expand All @@ -120,7 +121,7 @@ namespace AppInstaller::Repository::SQLite

void ParameterSpecificsImpl<blob_t>::Bind(sqlite3_stmt* stmt, int index, const blob_t& v)
{
THROW_IF_SQLITE_FAILED(sqlite3_bind_blob64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT));
THROW_IF_SQLITE_FAILED(sqlite3_bind_blob64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT), sqlite3_db_handle(stmt));
}

blob_t ParameterSpecificsImpl<blob_t>::GetColumn(sqlite3_stmt* stmt, int column)
Expand All @@ -143,22 +144,22 @@ namespace AppInstaller::Repository::SQLite
AICLI_LOG(SQL, Info, << "Opening SQLite connection: '" << target << "' [" << std::hex << static_cast<int>(disposition) << ", " << std::hex << static_cast<int>(flags) << "]");
// Always force connection serialization until we determine that there are situations where it is not needed
int resultingFlags = static_cast<int>(disposition) | static_cast<int>(flags) | SQLITE_OPEN_FULLMUTEX;
THROW_IF_SQLITE_FAILED(sqlite3_open_v2(target.c_str(), &m_dbconn, resultingFlags, nullptr));
THROW_IF_SQLITE_FAILED(sqlite3_open_v2(target.c_str(), &m_dbconn, resultingFlags, nullptr), nullptr);
}

Connection Connection::Create(const std::string& target, OpenDisposition disposition, OpenFlags flags)
{
Connection result{ target, disposition, flags };

THROW_IF_SQLITE_FAILED(sqlite3_extended_result_codes(result.m_dbconn.get(), 1));
THROW_IF_SQLITE_FAILED(sqlite3_extended_result_codes(result.m_dbconn.get(), 1), result.m_dbconn.get());

return result;
}

void Connection::EnableICU()
{
AICLI_LOG(SQL, Verbose, << "Enabling ICU");
THROW_IF_SQLITE_FAILED(sqlite3IcuInit(m_dbconn.get()));
THROW_IF_SQLITE_FAILED(sqlite3IcuInit(m_dbconn.get()), m_dbconn.get());
}

rowid_t Connection::GetLastInsertRowID()
Expand All @@ -177,7 +178,7 @@ namespace AppInstaller::Repository::SQLite
AICLI_LOG(SQL, Verbose, << "Preparing statement #" << m_id << ": " << sql);
// SQL string size should include the null terminator (https://www.sqlite.org/c3ref/prepare.html)
assert(sql.data()[sql.size()] == '\0');
THROW_IF_SQLITE_FAILED(sqlite3_prepare_v2(connection, sql.data(), static_cast<int>(sql.size() + 1), &m_stmt, nullptr));
THROW_IF_SQLITE_FAILED(sqlite3_prepare_v2(connection, sql.data(), static_cast<int>(sql.size() + 1), &m_stmt, nullptr), connection);
}

#if WINGET_SQLITE_EXPLAIN_QUERY_PLAN_ENABLED
Expand Down Expand Up @@ -264,7 +265,7 @@ namespace AppInstaller::Repository::SQLite
}
else
{
THROW_SQLITE(result);
THROW_SQLITE(result, sqlite3_db_handle(m_stmt.get()));
}
}
}
Expand Down