Skip to content
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
34 changes: 15 additions & 19 deletions include/database.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@

#pragma once

#include <stdexcept>
#include <memory>
#include <mutex>
#include <stdexcept>
#include <string>
#include <typeinfo>
#include <vector>

#include "fetch_query_results.h"
#include "queries.h"
#include "query_predicates.h"
#include "reflection.h"
Expand Down Expand Up @@ -71,8 +70,7 @@ class REFLECTION_EXPORT Database {
const auto type_id = typeid(T).name();
const auto& record = GetRecord(type_id);
EmptyPredicate empty;
const auto& query_result = Fetch(record, &empty);
return Hydrate<T>(query_result, record);
return FetchRecords<T>(record, &empty);
}

/// Retrieves all entries of a given record from the database, which match a given predicate.
Expand All @@ -81,8 +79,7 @@ class REFLECTION_EXPORT Database {
std::vector<T> Fetch(const QueryPredicateBase* predicate) const {
const auto type_id = typeid(T).name();
const auto& record = GetRecord(type_id);
const auto& query_result = Fetch(record, predicate);
return Hydrate<T>(query_result, record);
return FetchRecords<T>(record, predicate);
}

/// Retrieves a single entry of a given record from the database, which matches a given id.
Expand All @@ -92,11 +89,11 @@ class REFLECTION_EXPORT Database {
const auto type_id = typeid(T).name();
const auto& record = GetRecord(type_id);
Equal equal_id_condition(&T::id, id);
const auto& query_result = Fetch(record, &equal_id_condition);
if (query_result.row_values.size() != 1) {
auto models = FetchRecords<T>(record, &equal_id_condition);
if (models.size() != 1) {
throw std::runtime_error("No record with this id found");
}
return Hydrate<T>(query_result, record)[0];
return models[0];
}

/// Saves a given record in the database.
Expand Down Expand Up @@ -197,22 +194,21 @@ class REFLECTION_EXPORT Database {
private:
explicit Database(const char* path);

/// Executes a fetch query (SELECT) for a given record with a given predicate,
/// and returns the results in a textual representation
FetchQueryResults Fetch(const Reflection& record, const QueryPredicateBase* predicate) const;

/// Returns a record type from its type information, retrieved from typeid(...).name()
static const Reflection& GetRecord(const std::string& type_id);

/// Creates concrete record types with initialized members,
/// based on the textual representation of results from a fetch query
/// Executes a fetch query (SELECT) for a given record with a given predicate, streaming
/// each matching row directly from the prepared statement into a newly constructed T -
/// no intermediate string materialization of the result set
template <typename T>
std::vector<T> Hydrate(const FetchQueryResults& query_results, const Reflection& record) const {
std::vector<T> FetchRecords(const Reflection& record, const QueryPredicateBase* predicate) const {
std::lock_guard<std::mutex> lock(db_mutex_);
FetchRecordsQuery query(db_, record, predicate);
std::vector<T> models;
for (auto i = 0; i < query_results.row_values.size(); i++) {
while (query.StepRow()) {
T model;
FetchRecordsQuery::Hydrate((void*)&model, query_results, record, i);
models.emplace_back(model);
query.HydrateCurrentRow((void*)&model, record);
models.emplace_back(std::move(model));
}
return models;
}
Expand Down
33 changes: 0 additions & 33 deletions include/fetch_query_results.h

This file was deleted.

16 changes: 7 additions & 9 deletions include/queries.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,26 +139,24 @@ class REFLECTION_EXPORT UpdateQuery final : public ExecutionQuery {
void* p_;
};

struct FetchQueryResults;

/// A query for retrieving all records from the database, which match a given predicate condition
/// This maps to SELECT * in SQL
class REFLECTION_EXPORT FetchRecordsQuery final : public Query {
public:
explicit FetchRecordsQuery(sqlite3* db, const Reflection& record, const QueryPredicateBase* predicate);
~FetchRecordsQuery() override;

/// Returns a textual representation of the results of the query
FetchQueryResults GetResults();
/// Prepares the underlying statement on first use and advances it to the next matching
/// row. Returns false once there are no more rows left to fetch.
bool StepRow();

/// Reconstructs all record member values based on their concrete type,
/// based on the textual representation of the corresponding row result
/// of the fetch query
static void Hydrate(void* p, const FetchQueryResults& query_results, const Reflection& record, size_t i);
/// Hydrates the type-erased record pointed to by p from the current row of the prepared
/// statement, reading each column directly via its typed SQLite accessor, with no
/// intermediate string representation of the fetched values
void HydrateCurrentRow(void* p, const Reflection& record) const;

protected:
std::string PrepareSql() const override;
std::wstring GetColumnValue(int col) const;

sqlite3_stmt* stmt_;
const QueryPredicateBase* predicate_;
Expand Down
6 changes: 0 additions & 6 deletions src/database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,6 @@ std::shared_ptr<const Database> Database::Instance() {
return instance_;
}

FetchQueryResults Database::Fetch(const Reflection& record, const QueryPredicateBase* predicate) const {
std::lock_guard<std::mutex> lock(db_mutex_);
FetchRecordsQuery query(db_, record, predicate);
return query.GetResults();
}

const Reflection& Database::GetRecord(const std::string& type_id) {
return GetReflectionRegister().records.at(type_id);
}
Expand Down
2 changes: 0 additions & 2 deletions src/internal/string_utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ namespace sqlite_reflection {
/// between strings and concrete types used by SQLite
class REFLECTION_EXPORT StringUtilities {
public:
static int64_t ToInt(const std::wstring& s);
static std::string FromInt(int64_t value);

static double ToDouble(const std::wstring& s);
static std::string FromDouble(double value);

static std::string ToUtf8(const std::wstring& wide_string);
Expand Down
103 changes: 41 additions & 62 deletions src/queries.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@
#include <string.h>

#include <algorithm>
#include <cstdio>
#include <cstring>
#include <iterator>
#include <stdexcept>
#include <string>
#include <vector>

#include "fetch_query_results.h"
#include "internal/sqlite3.h"
#include "internal/string_utilities.h"

Expand Down Expand Up @@ -287,71 +285,77 @@ FetchRecordsQuery::~FetchRecordsQuery() {
}
}

FetchQueryResults FetchRecordsQuery::GetResults() {
const auto sql = PrepareSql();

if (sqlite3_prepare_v2(db_, sql.data(), -1, &stmt_, nullptr)) {
throw std::runtime_error((sql + ": could not get results").data());
}
BindValues(stmt_, predicate_->Bindings());

const auto column_count = sqlite3_column_count(stmt_);

FetchQueryResults results;
results.column_names.reserve(column_count);
for (auto i = 0; i < column_count; i++) {
results.column_names.emplace_back(sqlite3_column_name(stmt_, i));
}

while (sqlite3_step(stmt_) != SQLITE_DONE) {
std::vector<std::wstring> row;
row.reserve(column_count);
for (auto col = 0; col < column_count; col++) {
auto value = GetColumnValue(col);
row.emplace_back(value);
bool FetchRecordsQuery::StepRow() {
if (stmt_ == nullptr) {
const auto sql = PrepareSql();
if (sqlite3_prepare_v2(db_, sql.data(), -1, &stmt_, nullptr)) {
throw std::runtime_error((sql + ": could not get results").data());
}
results.row_values.emplace_back(row);
BindValues(stmt_, predicate_->Bindings());
}

return results;
return sqlite3_step(stmt_) != SQLITE_DONE;
}

void FetchRecordsQuery::Hydrate(void* p, const FetchQueryResults& query_results, const Reflection& record, size_t i) {
for (auto j = 0; j < query_results.column_names.size(); j++) {
const auto current_storage_class = record.member_metadata[j].storage_class;
const auto& content = query_results.row_values[i][j];
if (content.empty()) {
void FetchRecordsQuery::HydrateCurrentRow(void* p, const Reflection& record) const {
// Initialize() only ever runs CREATE TABLE IF NOT EXISTS, so a table that predates a
// field being added to the reflected struct can have fewer actual columns than
// record.member_metadata; indexing sqlite3_column_* past the statement's real column
// count is undefined behavior, so bound the loop by whichever is smaller, exactly as
// the prior text-based path did via sqlite3_column_count(stmt_)
const auto column_count = std::min(static_cast<size_t>(sqlite3_column_count(stmt_)), record.member_metadata.size());
for (size_t j = 0; j < column_count; j++) {
const auto col = static_cast<int>(j);

// The prior text-based path only ever produced a non-empty string for INTEGER,
// FLOAT, or TEXT columns; NULL and BLOB both fell through to an empty string and
// were skipped, regardless of the member's declared storage class. Replicate that
// here so a NULL or BLOB value (reachable via UnsafeSql or a foreign database file,
// even for a column declared as a different storage class) is never fed to the
// wrong typed accessor.
const int col_type = sqlite3_column_type(stmt_, col);
if (col_type == SQLITE_NULL || col_type == SQLITE_BLOB) {
continue;
}

const auto current_storage_class = record.member_metadata[j].storage_class;
switch (current_storage_class) {
case SqliteStorageClass::kInt: {
auto& v = *reinterpret_cast<int64_t*>(GetMemberAddress(p, record, j));
v = StringUtilities::ToInt(content);
v = sqlite3_column_int64(stmt_, col);
break;
}

case SqliteStorageClass::kBool: {
auto& v = *reinterpret_cast<bool*>(GetMemberAddress(p, record, j));
v = StringUtilities::ToInt(content) == 1;
v = sqlite3_column_int64(stmt_, col) == 1;
break;
}

case SqliteStorageClass::kReal: {
auto& v = *reinterpret_cast<double*>(GetMemberAddress(p, record, j));
v = StringUtilities::ToDouble(content);
v = sqlite3_column_double(stmt_, col);
break;
}

case SqliteStorageClass::kText: {
const auto byte_count = sqlite3_column_bytes(stmt_, col);
if (byte_count == 0) {
continue;
}
const auto content = reinterpret_cast<const char*>(sqlite3_column_text(stmt_, col));
auto& v = *reinterpret_cast<std::wstring*>(GetMemberAddress(p, record, j));
v = content;
v = StringUtilities::FromUtf8(content, byte_count);
Comment on lines +345 to +347

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve BLOB skip behavior for text hydration

When a reflected TEXT column contains a SQLite BLOB value, which can happen via UnsafeSql or an existing SQLite file despite the column affinity, this now feeds the raw blob bytes into FromUtf8. The removed GetColumnValue returned an empty string for SQLITE_BLOB and Hydrate skipped assignment, so Fetch previously tolerated those rows; invalid blob bytes now throw during fetch and prevent reading the row. The analogous DATETIME branch below has the same issue, so keep the old skip behavior for BLOBs before converting.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, confirmed by reproducing it: reverting the guard reproduced exactly this - std::range_error from wstring_convert::from_bytes on invalid-UTF8 blob bytes, plus silent 0/garbage assignment on the INT/BOOL/REAL branches for the same root cause (they weren't mentioned above but have the identical issue, since the old GetColumnValue treated BLOB the same as NULL for every storage class, not just TEXT/DATETIME).

Fixed in b225f62 by extending the skip check to SQLITE_NULL || SQLITE_BLOB up front, which matches the old code's default: branch exactly for all five storage classes in one place. Added a regression test with an invalid-UTF8 blob in a TEXT-affinity column.


Generated by Claude Code

break;
}

case SqliteStorageClass::kDateTime: {
const auto byte_count = sqlite3_column_bytes(stmt_, col);
if (byte_count == 0) {
continue;
}
const auto content = reinterpret_cast<const char*>(sqlite3_column_text(stmt_, col));
auto& v = *reinterpret_cast<TimePoint*>(GetMemberAddress(p, record, j));
v = TimePoint::FromSystemTime(content);
v = TimePoint::FromSystemTime(StringUtilities::FromUtf8(content, byte_count));
break;
}

Expand All @@ -370,28 +374,3 @@ std::string FetchRecordsQuery::PrepareSql() const {
}
return sql + ";";
}

std::wstring FetchRecordsQuery::GetColumnValue(const int col) const {
const int col_type = sqlite3_column_type(stmt_, col);
switch (col_type) {
case SQLITE_INTEGER:
return std::to_wstring(sqlite3_column_int64(stmt_, col));

case SQLITE_FLOAT: {
// %.17g round-trips any double exactly; to_wstring's fixed 6-decimal
// formatting would silently truncate precision here
char buffer[64];
std::snprintf(buffer, sizeof(buffer), "%.17g", sqlite3_column_double(stmt_, col));
return StringUtilities::FromUtf8(buffer, std::strlen(buffer));
}

case SQLITE_TEXT: {
const auto content = reinterpret_cast<const char*>(sqlite3_column_text(stmt_, col));
const auto byte_count = sqlite3_column_bytes(stmt_, col);
return StringUtilities::FromUtf8(content, byte_count);
}

default:
return L"";
}
}
16 changes: 0 additions & 16 deletions src/string_utilities.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,10 @@

using namespace sqlite_reflection;

int64_t StringUtilities::ToInt(const std::wstring& s) {
try {
return std::stoll(s);
} catch (...) {
return 0;
}
}

std::string StringUtilities::FromInt(int64_t value) {
return std::to_string(value);
}

double StringUtilities::ToDouble(const std::wstring& s) {
try {
return std::stod(s);
} catch (...) {
return 0.0;
}
}

std::string StringUtilities::FromDouble(double value) {
auto textual_representation = std::to_string(value);
if (textual_representation.find('.') != std::string::npos) {
Expand Down
Loading
Loading