From cfaa74710c2a9462b4243a1506b295de8734f559 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 20:10:11 +0000 Subject: [PATCH 1/4] Hydrate fetched rows directly from typed SQLite columns, removing the wstring round-trip (#17) FetchRecordsQuery::GetResults stringified every fetched column into a std::wstring and materialized the whole result set as a vector> (FetchQueryResults) before Hydrate parsed each string back into the typed member. This is the performance/memory half of #17 (the correctness half - int64 truncation and REAL precision loss - was already fixed in #30). Replace the two-pass string round-trip with row-by-row direct hydration: FetchRecordsQuery now exposes StepRow() (advance the prepared statement) and HydrateCurrentRow() (read the current row straight into the destination member via sqlite3_column_int64/_double/_text+_bytes). Database::FetchRecords owns the construction loop, materializing only the final vector - the string double-buffer and per-value wstring allocation are both gone. FetchQueryResults, GetResults, GetColumnValue and the string-based Hydrate are removed as they're no longer needed. NULL/empty-string skip semantics are preserved exactly: a NULL column of any storage class is left unassigned, and an empty TEXT column is likewise left unassigned, matching the prior text-path behavior bit for bit (the NULL vs empty-string conflation itself is issue #20 and remains out of scope). Benchmarked FetchAll() over 100k rows against the pre-refactor build: ~2.9x faster wall-clock (225ms -> 77ms) and ~6x fewer allocations (2.4M -> 400k). Added a 50k-row round-trip test and a dedicated NULL/empty-string semantics test; full suite (62 tests) passes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NUt3c1wdseRtRSSXfg3MCK --- include/database.h | 34 ++++++------- include/fetch_query_results.h | 33 ------------- include/queries.h | 16 +++--- src/database.cc | 6 --- src/queries.cc | 93 ++++++++++++----------------------- tests/database_test.cc | 61 +++++++++++++++++++++++ 6 files changed, 114 insertions(+), 129 deletions(-) delete mode 100644 include/fetch_query_results.h diff --git a/include/database.h b/include/database.h index 611bdb7..0c5b9a5 100644 --- a/include/database.h +++ b/include/database.h @@ -22,14 +22,13 @@ #pragma once -#include #include #include +#include #include #include #include -#include "fetch_query_results.h" #include "queries.h" #include "query_predicates.h" #include "reflection.h" @@ -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(query_result, record); + return FetchRecords(record, &empty); } /// Retrieves all entries of a given record from the database, which match a given predicate. @@ -81,8 +79,7 @@ class REFLECTION_EXPORT Database { std::vector 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(query_result, record); + return FetchRecords(record, predicate); } /// Retrieves a single entry of a given record from the database, which matches a given id. @@ -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(record, &equal_id_condition); + if (models.size() != 1) { throw std::runtime_error("No record with this id found"); } - return Hydrate(query_result, record)[0]; + return models[0]; } /// Saves a given record in the database. @@ -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 - std::vector Hydrate(const FetchQueryResults& query_results, const Reflection& record) const { + std::vector FetchRecords(const Reflection& record, const QueryPredicateBase* predicate) const { + std::lock_guard lock(db_mutex_); + FetchRecordsQuery query(db_, record, predicate); std::vector 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; } diff --git a/include/fetch_query_results.h b/include/fetch_query_results.h deleted file mode 100644 index c932e52..0000000 --- a/include/fetch_query_results.h +++ /dev/null @@ -1,33 +0,0 @@ -// MIT License -// -// Copyright (c) 2026 Ioannis Kaliakatsos -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -#pragma once -#include -#include - -namespace sqlite_reflection { -/// A wrapper of the results of an SQLite SELECT query -struct FetchQueryResults { - std::vector column_names; - std::vector> row_values; -}; -} // namespace sqlite_reflection diff --git a/include/queries.h b/include/queries.h index b0929af..23a2ca0 100644 --- a/include/queries.h +++ b/include/queries.h @@ -139,8 +139,6 @@ 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 { @@ -148,17 +146,17 @@ class REFLECTION_EXPORT FetchRecordsQuery final : public Query { 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_; diff --git a/src/database.cc b/src/database.cc index 870258b..a5feea7 100644 --- a/src/database.cc +++ b/src/database.cc @@ -101,12 +101,6 @@ std::shared_ptr Database::Instance() { return instance_; } -FetchQueryResults Database::Fetch(const Reflection& record, const QueryPredicateBase* predicate) const { - std::lock_guard 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); } diff --git a/src/queries.cc b/src/queries.cc index 9acde85..771d73d 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -25,14 +25,12 @@ #include #include -#include #include #include #include #include #include -#include "fetch_query_results.h" #include "internal/sqlite3.h" #include "internal/string_utilities.h" @@ -287,71 +285,67 @@ 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 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 { + const auto column_count = record.member_metadata.size(); + for (size_t j = 0; j < column_count; j++) { + const auto col = static_cast(j); + + // A SQL NULL is left unset (member keeps its default value), regardless of the + // member's declared storage class + if (sqlite3_column_type(stmt_, col) == SQLITE_NULL) { continue; } + const auto current_storage_class = record.member_metadata[j].storage_class; switch (current_storage_class) { case SqliteStorageClass::kInt: { auto& v = *reinterpret_cast(GetMemberAddress(p, record, j)); - v = StringUtilities::ToInt(content); + v = sqlite3_column_int64(stmt_, col); break; } case SqliteStorageClass::kBool: { auto& v = *reinterpret_cast(GetMemberAddress(p, record, j)); - v = StringUtilities::ToInt(content) == 1; + v = sqlite3_column_int64(stmt_, col) == 1; break; } case SqliteStorageClass::kReal: { auto& v = *reinterpret_cast(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(sqlite3_column_text(stmt_, col)); auto& v = *reinterpret_cast(GetMemberAddress(p, record, j)); - v = content; + v = StringUtilities::FromUtf8(content, byte_count); break; } case SqliteStorageClass::kDateTime: { + const auto byte_count = sqlite3_column_bytes(stmt_, col); + if (byte_count == 0) { + continue; + } + const auto content = reinterpret_cast(sqlite3_column_text(stmt_, col)); auto& v = *reinterpret_cast(GetMemberAddress(p, record, j)); - v = TimePoint::FromSystemTime(content); + v = TimePoint::FromSystemTime(StringUtilities::FromUtf8(content, byte_count)); break; } @@ -370,28 +364,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(sqlite3_column_text(stmt_, col)); - const auto byte_count = sqlite3_column_bytes(stmt_, col); - return StringUtilities::FromUtf8(content, byte_count); - } - - default: - return L""; - } -} diff --git a/tests/database_test.cc b/tests/database_test.cc index 0c64bfc..6e748ae 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -613,6 +613,67 @@ TEST_F(DatabaseTest, FetchPreservesHighPrecisionDoubleValues) { EXPECT_DOUBLE_EQ(large_magnitude, fetched_second.salary); } +TEST_F(DatabaseTest, FetchPreservesNullAndEmptyStringSkipSemantics) { + const auto db = Database::Instance(); + + // Columns omitted from a raw INSERT are stored as SQL NULL. Direct hydration must skip + // assignment for a NULL column, and must also skip assignment for a TEXT column holding + // a genuine empty string - matching the pre-refactor behavior exactly. Both are only + // observable here for wstring members: std::wstring's default constructor deterministically + // produces an empty string regardless of whether it was assigned, whereas a skipped + // scalar member (e.g. an omitted INTEGER column) keeps whatever indeterminate value + // T's default construction happens to leave it at - both before and after this refactor - + // so that case isn't asserted on here. Resolving the NULL/empty-string conflation itself + // is tracked separately as #20 and is out of scope here. + db->UnsafeSql("INSERT INTO Company (id, name, age, salary) VALUES (1, '', 30, 50000.0)"); + db->UnsafeSql("INSERT INTO Company (id, age, address, salary) VALUES (2, 31, 'Nowhere', 60000.0)"); + + const auto first = db->Fetch(1); + EXPECT_EQ(L"", first.name); // explicit empty string + EXPECT_EQ(L"", first.address); // omitted column -> SQL NULL + EXPECT_EQ(30, first.age); + EXPECT_EQ(50000.0, first.salary); + + const auto second = db->Fetch(2); + EXPECT_EQ(L"", second.name); // omitted column -> SQL NULL + EXPECT_EQ(L"Nowhere", second.address); + EXPECT_EQ(31, second.age); + EXPECT_EQ(60000.0, second.salary); +} + +TEST_F(DatabaseTest, FetchAllRoundTripsLargeBatchExactly) { + const auto db = Database::Instance(); + + // Correctness-at-scale: a large FetchAll must still hydrate every row exactly, now that + // hydration reads directly from the prepared statement instead of materializing the + // whole result set as strings first + constexpr int kRowCount = 50000; + std::vector companies; + companies.reserve(kRowCount); + for (int i = 0; i < kRowCount; ++i) { + Company c; + c.id = i + 1; + c.name = L"company_" + std::to_wstring(i); + c.age = 5000000000LL + i; // beyond INT32_MAX for every row + c.address = L"address_" + std::to_wstring(i); + c.salary = 0.1 + static_cast(i) * 1e-9; // needs full double precision + companies.push_back(c); + } + + db->Save(companies); + + const auto fetched = db->FetchAll(); + ASSERT_EQ(static_cast(kRowCount), fetched.size()); + + for (int i = 0; i < kRowCount; ++i) { + EXPECT_EQ(companies[i].id, fetched[i].id); + EXPECT_EQ(companies[i].name, fetched[i].name); + EXPECT_EQ(companies[i].age, fetched[i].age); + EXPECT_EQ(companies[i].address, fetched[i].address); + EXPECT_DOUBLE_EQ(companies[i].salary, fetched[i].salary); + } +} + TEST_F(DatabaseTest, RawSqlQueryForPersistedRecord) { const auto db = Database::Instance(); From 7f5b467716295210102a024364cd0679ebaa4561 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 20:12:23 +0000 Subject: [PATCH 2/4] Remove now-unused StringUtilities::ToInt/ToDouble Direct hydration (previous commit) reads ints and doubles straight from the prepared statement via sqlite3_column_int64/_double, so these two string-parsing helpers no longer have any callers. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NUt3c1wdseRtRSSXfg3MCK --- src/internal/string_utilities.h | 2 -- src/string_utilities.cc | 16 ---------------- 2 files changed, 18 deletions(-) diff --git a/src/internal/string_utilities.h b/src/internal/string_utilities.h index 5e97a25..183ef99 100644 --- a/src/internal/string_utilities.h +++ b/src/internal/string_utilities.h @@ -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); diff --git a/src/string_utilities.cc b/src/string_utilities.cc index 9622a1b..c7f6d1c 100644 --- a/src/string_utilities.cc +++ b/src/string_utilities.cc @@ -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) { From b225f624da01c9aabf87170bda59c3ea408932db Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 20:20:27 +0000 Subject: [PATCH 3/4] Skip BLOB columns during hydration, matching the pre-refactor behavior (#17) The removed text-based path returned an empty string for any column whose runtime SQLite type wasn't INTEGER/FLOAT/TEXT, so both NULL and BLOB columns were skipped during Hydrate regardless of the member's declared storage class. Direct hydration only replicated the NULL half of that: a BLOB value (reachable via UnsafeSql or a foreign database file, even for a column declared as a different storage class) fell through to the typed accessors unconditionally, which could throw out of StringUtilities::FromUtf8 for invalid UTF-8 blob bytes on TEXT/DATETIME members, and silently assigned 0/0.0 instead of skipping for INT/BOOL/REAL members. Extend the skip check to SQLITE_NULL || SQLITE_BLOB, matching the old GetColumnValue's default case exactly. Adds a regression test inserting an invalid-UTF8 BLOB into a TEXT-affinity column and asserting Fetch neither throws nor corrupts the other members. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NUt3c1wdseRtRSSXfg3MCK --- src/queries.cc | 11 ++++++++--- tests/database_test.cc | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/queries.cc b/src/queries.cc index 771d73d..7725f63 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -301,9 +301,14 @@ void FetchRecordsQuery::HydrateCurrentRow(void* p, const Reflection& record) con for (size_t j = 0; j < column_count; j++) { const auto col = static_cast(j); - // A SQL NULL is left unset (member keeps its default value), regardless of the - // member's declared storage class - if (sqlite3_column_type(stmt_, col) == SQLITE_NULL) { + // 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; } diff --git a/tests/database_test.cc b/tests/database_test.cc index 6e748ae..545a805 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -641,6 +641,26 @@ TEST_F(DatabaseTest, FetchPreservesNullAndEmptyStringSkipSemantics) { EXPECT_EQ(60000.0, second.salary); } +TEST_F(DatabaseTest, FetchSkipsBlobColumnsWithoutThrowing) { + const auto db = Database::Instance(); + + // A BLOB value (reachable via raw SQL despite the column's declared affinity - here an + // invalid UTF-8 byte) must be skipped exactly like a NULL, not fed into the typed + // accessor for the member's declared storage class. The old text-based path returned an + // empty string for any column whose runtime type wasn't INTEGER/FLOAT/TEXT (i.e. NULL or + // BLOB) and Hydrate skipped assignment on that; direct hydration must replicate this or + // else invalid UTF-8 blob bytes would throw out of FromUtf8 during a TEXT member's + // hydration, where the old path silently tolerated the row. + db->UnsafeSql("INSERT INTO Company (id, name, age, address, salary) VALUES (1, X'FF', 30, 'Nowhere', 50000.0)"); + + Company fetched; + EXPECT_NO_THROW(fetched = db->Fetch(1)); + EXPECT_EQ(L"", fetched.name); + EXPECT_EQ(L"Nowhere", fetched.address); + EXPECT_EQ(30, fetched.age); + EXPECT_EQ(50000.0, fetched.salary); +} + TEST_F(DatabaseTest, FetchAllRoundTripsLargeBatchExactly) { const auto db = Database::Instance(); From cf92b991f07a2ce202a405f8461f88ba2bc2e3bc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 20:30:36 +0000 Subject: [PATCH 4/4] Bound hydration by the statement's actual column count (#17) Database::Initialize only ever runs CREATE TABLE IF NOT EXISTS, so a table created by an older version of a reflected struct (before a field was added) is never migrated - its actual column count can be smaller than record.member_metadata.size(). The removed text-based path bounded its loop by sqlite3_column_count(stmt_) (the statement's real column count); HydrateCurrentRow used record.member_metadata.size() instead, which can index sqlite3_column_* past the end of the result row - undefined behavior per SQLite's docs, even though the vendored SQLite build happens to guard against it defensively today. Bound the loop by min(sqlite3_column_count(stmt_), member_metadata.size()), matching the old behavior exactly. Adds a regression test that drops columns from an existing table (simulating schema drift) and fetches successfully. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NUt3c1wdseRtRSSXfg3MCK --- src/queries.cc | 7 ++++++- tests/database_test.cc | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/queries.cc b/src/queries.cc index 7725f63..5148b67 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -297,7 +297,12 @@ bool FetchRecordsQuery::StepRow() { } void FetchRecordsQuery::HydrateCurrentRow(void* p, const Reflection& record) const { - const auto column_count = record.member_metadata.size(); + // 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(sqlite3_column_count(stmt_)), record.member_metadata.size()); for (size_t j = 0; j < column_count; j++) { const auto col = static_cast(j); diff --git a/tests/database_test.cc b/tests/database_test.cc index 545a805..ecdbcbf 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -661,6 +661,25 @@ TEST_F(DatabaseTest, FetchSkipsBlobColumnsWithoutThrowing) { EXPECT_EQ(50000.0, fetched.salary); } +TEST_F(DatabaseTest, FetchToleratesTableWithFewerColumnsThanCurrentStruct) { + const auto db = Database::Instance(); + + // Initialize() only ever runs CREATE TABLE IF NOT EXISTS, so a table created by an older + // version of a reflected struct is never migrated to add newly introduced columns. + // Simulate that drift directly (rather than needing a pre-existing file) by dropping + // columns after the row is saved: SELECT * then returns fewer columns than + // record.member_metadata.size(), which must not read past the statement's real column + // count when hydrating. + db->Save(Company{L"Old Corp", 40, L"Nowhere", 12345.0, 1}); + db->UnsafeSql("ALTER TABLE Company DROP COLUMN address"); + db->UnsafeSql("ALTER TABLE Company DROP COLUMN salary"); + + Company fetched; + EXPECT_NO_THROW(fetched = db->Fetch(1)); + EXPECT_EQ(L"Old Corp", fetched.name); + EXPECT_EQ(40, fetched.age); +} + TEST_F(DatabaseTest, FetchAllRoundTripsLargeBatchExactly) { const auto db = Database::Instance();