From 383f75d76bf94647007ea3bb612a2ec0578b9b31 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 05:08:32 +0000 Subject: [PATCH 1/7] Fix SaveAutoIncrement to write back the database-assigned id (#6) SaveAutoIncrement previously copied the input record, set an id computed client-side as MAX(id)+1, and persisted the copy, leaving the caller's object without the generated id and with no way to look the row back up. SaveAutoIncrement now takes a mutable reference and lets SQLite assign the id natively: the INSERT omits the id column (which is an INTEGER PRIMARY KEY / rowid alias) and the value returned by sqlite3_last_insert_rowid is written back into the passed-in model. This is atomic, avoids an extra SELECT MAX per save, and removes the stale "id is not modified" behavior described in the issue. - InsertQuery gains an auto_increment_id mode that drops the id column and its binding from the statement - Database::SaveAutoIncrement(void*, record) returns the assigned id - Public templates updated to T& / vector& and write the id back - Tests assert the id is populated on single and batch saves and that ids continue from the current maximum - README updated to document the new write-back behavior --- README.md | 4 ++- include/database.h | 67 ++++++++++++++++-------------------------- include/queries.h | 6 ++-- src/database.cc | 6 ++++ src/queries.cc | 25 +++++++++++++--- tests/database_test.cc | 42 +++++++++++++++++++++++++- 6 files changed, 101 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 4a1fb50..1df9b69 100644 --- a/README.md +++ b/README.md @@ -182,18 +182,20 @@ people.push_back({L"Jane", L"Doe", 41, true, 7}); db.Save(people); ``` -Use `SaveAutoIncrement` when you want the library to assign the next available `id` for the saved row. The input object is copied; its `id` member is not modified. +Use `SaveAutoIncrement` when you want the library to assign the next available `id` for the saved row. The generated `id` is written back into the passed-in object, so it must be a mutable (non-`const`) lvalue. ```c++ // Omit the id and let sqlite-reflection assign the next available value. Person new_person{L"John", L"Doe", 28, false}; db.SaveAutoIncrement(new_person); +// new_person.id now holds the value assigned by the database. // The same auto-increment behavior is available for batches. std::vector more_people; more_people.push_back({L"Ada", L"Lovelace", 36, true}); more_people.push_back({L"Grace", L"Hopper", 85, true}); db.SaveAutoIncrement(more_people); +// Each element's id is now populated. ``` ## Fetching records diff --git a/include/database.h b/include/database.h index 19d02f7..3356f59 100644 --- a/include/database.h +++ b/include/database.h @@ -107,28 +107,42 @@ class REFLECTION_EXPORT Database { /// This corresponds to an INSERT query in the SQL syntax template void Save(const T& model) const { - Save(model, false); + const auto type_id = typeid(T).name(); + const auto& record = GetRecord(type_id); + Save((void*)&model, record); } - /// Saves a given record in the database and auto-increments its id. + /// Saves a given record in the database, letting the database assign its id. + /// The newly generated id is written back into the passed-in model. /// This corresponds to an INSERT query in the SQL syntax template - void SaveAutoIncrement(const T& model) const { - Save(model, true); + void SaveAutoIncrement(T& model) const { + const auto type_id = typeid(T).name(); + const auto& record = GetRecord(type_id); + model.id = SaveAutoIncrement((void*)&model, record); } /// Saves multiple records iteratively in the database. /// This corresponds to an INSERT query in the SQL syntax template void Save(const std::vector& models) const { - Save(models, false); + const auto type_id = typeid(T).name(); + const auto& record = GetRecord(type_id); + for (const auto& model : models) { + Save((void*)&model, record); + } } - /// Saves multiple records iteratively in the database and auto-increments their ids. + /// Saves multiple records iteratively in the database, letting the database assign their ids. + /// The newly generated ids are written back into the passed-in models. /// This corresponds to an INSERT query in the SQL syntax template - void SaveAutoIncrement(const std::vector& models) const { - Save(models, true); + void SaveAutoIncrement(std::vector& models) const { + const auto type_id = typeid(T).name(); + const auto& record = GetRecord(type_id); + for (auto& model : models) { + model.id = SaveAutoIncrement((void*)&model, record); + } } /// Updates a given record in the database. @@ -207,42 +221,13 @@ class REFLECTION_EXPORT Database { return models; } - /// Saves a given record in the database. - /// This corresponds to an INSERT query in the SQL syntax - template - void Save(const T& model, bool auto_increment_id) const { - const auto type_id = typeid(T).name(); - const auto& record = GetRecord(type_id); - T saved_model(model); - if (auto_increment_id) { - const auto current_max_id = GetMaxId(); - saved_model.id = current_max_id + 1; - } - Save((void*)&saved_model, record); - } - - /// Saves multiple records iteratively in the database. - /// This corresponds to an INSERT query in the SQL syntax - template - void Save(const std::vector& models, bool auto_increment_id) const { - const auto type_id = typeid(T).name(); - const auto& record = GetRecord(type_id); - const auto current_max_id = auto_increment_id ? GetMaxId() : 0; - for (auto i = 0; i < models.size(); ++i) { - const auto& model = models[i]; - if (auto_increment_id) { - T saved_model(model); - saved_model.id = current_max_id + i + 1; - Save((void*)&saved_model, record); - } else { - Save((void*)&model, record); - } - } - } - /// Saves a single record in the database void Save(void* p, const Reflection& record) const; + /// Saves a single record in the database, letting SQLite assign its id, + /// and returns the id that was generated for the inserted row + int64_t SaveAutoIncrement(void* p, const Reflection& record) const; + /// Updates a single record in the database void Update(void* p, const Reflection& record) const; diff --git a/include/queries.h b/include/queries.h index c53feed..8605f50 100644 --- a/include/queries.h +++ b/include/queries.h @@ -112,16 +112,18 @@ class REFLECTION_EXPORT DeleteQuery final : public ExecutionQuery { }; /// A query to insert a given record to the database, by supplying a given type-erased struct instance -/// This maps to INSERT INTO in SQL +/// This maps to INSERT INTO in SQL. When auto_increment_id is set, the id column is omitted from the +/// statement so that SQLite assigns the next rowid value itself. class REFLECTION_EXPORT InsertQuery final : public ExecutionQuery { public: ~InsertQuery() override = default; - explicit InsertQuery(sqlite3* db, const Reflection& record, void* p); + explicit InsertQuery(sqlite3* db, const Reflection& record, void* p, bool auto_increment_id = false); protected: std::string PrepareSql() const override; std::vector Bindings() const override; void* p_; + bool auto_increment_id_; }; /// A query to update a given record to the database, by supplying a given type-erased struct instance diff --git a/src/database.cc b/src/database.cc index 040cf3c..913f50d 100644 --- a/src/database.cc +++ b/src/database.cc @@ -82,6 +82,12 @@ void Database::Save(void* p, const Reflection& record) const { query.Execute(); } +int64_t Database::SaveAutoIncrement(void* p, const Reflection& record) const { + InsertQuery query(db_, record, p, true); + query.Execute(); + return sqlite3_last_insert_rowid(db_); +} + void Database::Update(void* p, const Reflection& record) const { UpdateQuery query(db_, record, p); query.Execute(); diff --git a/src/queries.cc b/src/queries.cc index f9d1ac9..9c6a42b 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -210,17 +210,34 @@ std::vector DeleteQuery::Bindings() const { return predicate_->Bindings(); } -InsertQuery::InsertQuery(sqlite3* db, const Reflection& record, void* p) : ExecutionQuery(db, record), p_(p) {} +InsertQuery::InsertQuery(sqlite3* db, const Reflection& record, void* p, bool auto_increment_id) + : ExecutionQuery(db, record), p_(p), auto_increment_id_(auto_increment_id) {} std::string InsertQuery::PrepareSql() const { std::string sql("INSERT INTO "); - sql += record_.name + " (" + JoinedRecordColumnNames() + ") VALUES ("; - sql += Placeholders(record_.member_metadata.size()) + ");"; + sql += record_.name + " ("; + + if (auto_increment_id_) { + // Omit the id column (index 0) so SQLite assigns the next rowid itself + auto columns = GetRecordColumnNames(); + const std::vector columns_without_id(columns.begin() + 1, columns.end()); + sql += StringUtilities::Join(columns_without_id, ", ") + ") VALUES ("; + sql += Placeholders(columns_without_id.size()) + ");"; + } else { + sql += JoinedRecordColumnNames() + ") VALUES ("; + sql += Placeholders(record_.member_metadata.size()) + ");"; + } + return sql; } std::vector InsertQuery::Bindings() const { - return GetValues(p_); + auto values = GetValues(p_); + if (auto_increment_id_) { + // The id column is omitted from the statement, so drop its binding too + values.erase(values.begin()); + } + return values; } UpdateQuery::UpdateQuery(sqlite3* db, const Reflection& record, void* p) : ExecutionQuery(db, record), p_(p) {} diff --git a/tests/database_test.cc b/tests/database_test.cc index cf97389..8ecdc33 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -68,9 +68,12 @@ TEST_F(DatabaseTest, SingleInsertion) { TEST_F(DatabaseTest, SingleInsertionWithAutoIdIncrement) { const auto& db = Database::Instance(); - const Person p{L"παναγιώτης", L"ανδριανόπουλος", 39}; + Person p{L"παναγιώτης", L"ανδριανόπουλος", 39}; db.SaveAutoIncrement(p); + // The generated id is written back into the passed-in record + EXPECT_EQ(1, p.id); + const auto all_persons = db.FetchAll(); EXPECT_EQ(1, all_persons.size()); @@ -80,6 +83,43 @@ TEST_F(DatabaseTest, SingleInsertionWithAutoIdIncrement) { EXPECT_EQ(1, all_persons[0].id); } +TEST_F(DatabaseTest, MultipleInsertionsWithAutoIdIncrement) { + const auto& db = Database::Instance(); + + std::vector persons; + persons.push_back({L"παναγιώτης", L"ανδριανόπουλος", 28}); + persons.push_back({L"peter", L"meier", 32}); + + db.SaveAutoIncrement(persons); + + // The generated ids are written back into the passed-in records + EXPECT_EQ(1, persons[0].id); + EXPECT_EQ(2, persons[1].id); + + const auto saved_persons = db.FetchAll(); + EXPECT_EQ(2, saved_persons.size()); + + for (auto i = 0; i < saved_persons.size(); ++i) { + EXPECT_EQ(persons[i].id, saved_persons[i].id); + EXPECT_EQ(persons[i].first_name, saved_persons[i].first_name); + EXPECT_EQ(persons[i].last_name, saved_persons[i].last_name); + EXPECT_EQ(persons[i].age, saved_persons[i].age); + } +} + +TEST_F(DatabaseTest, AutoIdIncrementContinuesFromExistingMaxId) { + const auto& db = Database::Instance(); + + const Person existing{L"ada", L"lovelace", 36, true, 10}; + db.Save(existing); + + Person p{L"grace", L"hopper", 85}; + db.SaveAutoIncrement(p); + + // The new id continues from the current maximum id in the table + EXPECT_EQ(11, p.id); +} + TEST_F(DatabaseTest, MultipleInsertions) { const auto& db = Database::Instance(); From 09b5eb2d45dfac367b2aa6b1e5af261a8aa80aa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 05:11:23 +0000 Subject: [PATCH 2/7] Use AUTOINCREMENT so generated ids are never reused (#6) Plain INTEGER PRIMARY KEY reuses the rowid of a deleted highest row on the next insert. Declaring the id column as INTEGER PRIMARY KEY AUTOINCREMENT makes SQLite track the highest id ever assigned in sqlite_sequence and never hand it out again, which is the expected behavior for stable record identifiers. - CreateTableQuery emits PRIMARY KEY AUTOINCREMENT for the id column - Added a test asserting an id is not reused after the highest row is deleted - README documents the no-reuse guarantee --- README.md | 2 +- src/queries.cc | 4 +++- tests/database_test.cc | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1df9b69..522df35 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ people.push_back({L"Jane", L"Doe", 41, true, 7}); db.Save(people); ``` -Use `SaveAutoIncrement` when you want the library to assign the next available `id` for the saved row. The generated `id` is written back into the passed-in object, so it must be a mutable (non-`const`) lvalue. +Use `SaveAutoIncrement` when you want the library to assign the next available `id` for the saved row. The database generates the `id` (via an `AUTOINCREMENT` column, so values are never reused even after rows are deleted) and it is written back into the passed-in object, which must therefore be a mutable (non-`const`) lvalue. ```c++ // Omit the id and let sqlite-reflection assign the next available value. diff --git a/src/queries.cc b/src/queries.cc index 9c6a42b..3e2e193 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -194,7 +194,9 @@ std::string CreateTableQuery::CustomizedColumnName(size_t index) const { const auto is_id = name.compare(std::string("id")) == 0; name += " " + record_.member_metadata[index].sqlite_column_name; - return is_id ? name + " PRIMARY KEY" : name; + // AUTOINCREMENT guarantees that ids are never reused, even after the row with the + // highest id is deleted + return is_id ? name + " PRIMARY KEY AUTOINCREMENT" : name; } DeleteQuery::DeleteQuery(sqlite3* db, const Reflection& record, const QueryPredicateBase* predicate) diff --git a/tests/database_test.cc b/tests/database_test.cc index 8ecdc33..18cf88e 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -120,6 +120,26 @@ TEST_F(DatabaseTest, AutoIdIncrementContinuesFromExistingMaxId) { EXPECT_EQ(11, p.id); } +TEST_F(DatabaseTest, AutoIdIsNotReusedAfterDeletingHighestRow) { + const auto& db = Database::Instance(); + + Person first{L"ada", L"lovelace", 36}; + db.SaveAutoIncrement(first); + EXPECT_EQ(1, first.id); + + Person second{L"grace", L"hopper", 85}; + db.SaveAutoIncrement(second); + EXPECT_EQ(2, second.id); + + // Remove the row with the highest id + db.Delete(second); + + // A subsequent auto-increment save must not reuse the freed id + Person third{L"john", L"doe", 28}; + db.SaveAutoIncrement(third); + EXPECT_EQ(3, third.id); +} + TEST_F(DatabaseTest, MultipleInsertions) { const auto& db = Database::Instance(); From e78b0cb3c64424dcaa28d4cced99e44da2c0b86b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 05:36:07 +0000 Subject: [PATCH 3/7] ci: pin Windows jobs to windows-2022 runner The windows-latest runner image no longer provides a Visual Studio 2022 (v17) instance that CMake can detect, so configuration failed with "could not find any instance of Visual Studio" for the hardcoded "Visual Studio 17 2022" generator. Pinning the Windows matrix entries to windows-2022 guarantees a matching toolchain is present. --- .github/workflows/cmake.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index d4dd301..b9efcb7 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -22,7 +22,7 @@ jobs: config: - { name: "Windows Latest MSVC (C++11)", - os: windows-latest, + os: windows-2022, build_type: "Debug", cc: "cl", cxx: "cl", @@ -31,7 +31,7 @@ jobs: } - { name: "Windows Latest MSVC (C++20)", - os: windows-latest, + os: windows-2022, build_type: "Debug", cc: "cl", cxx: "cl", From 91864d5a1131d5e932e5c8e40110100cbf96925c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 05:47:13 +0000 Subject: [PATCH 4/7] docs: qualify the never-reused id guarantee for existing databases The AUTOINCREMENT guarantee is applied at table creation. Since tables are created with CREATE TABLE IF NOT EXISTS, databases created by an earlier release keep their original id INTEGER PRIMARY KEY schema until recreated or migrated. Document this caveat in the README so the guarantee is not overstated. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 522df35..089941f 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,8 @@ db.Save(people); Use `SaveAutoIncrement` when you want the library to assign the next available `id` for the saved row. The database generates the `id` (via an `AUTOINCREMENT` column, so values are never reused even after rows are deleted) and it is written back into the passed-in object, which must therefore be a mutable (non-`const`) lvalue. +> **Note:** the never-reused guarantee relies on the `id` column being declared `AUTOINCREMENT`, which is applied when a table is created. Because tables are created with `CREATE TABLE IF NOT EXISTS`, the guarantee only holds for tables created by this version onwards. A database file created by an earlier release keeps its original `id INTEGER PRIMARY KEY` schema (where a deleted highest id can be reused) until the table is recreated or migrated. + ```c++ // Omit the id and let sqlite-reflection assign the next available value. Person new_person{L"John", L"Doe", 28, false}; From 62625ff9e5b8b4537c9c51b14f2f7ccced8cfcd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 06:02:13 +0000 Subject: [PATCH 5/7] Fix auto-increment insert for records with only an id column When a reflected record has no fields besides the implicit id, the auto-increment insert path produced "INSERT INTO Name () VALUES ()", which SQLite rejects as a syntax error. Emit "INSERT INTO Name DEFAULT VALUES" (with no bindings) for that case so the row is inserted and the rowid is assigned. Adds an id-only reflectable test record and a regression test covering auto-increment insertion and fetch for it. --- src/queries.cc | 14 ++++++++++---- tests/database_test.cc | 20 ++++++++++++++++++++ tests/id_only_record.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 tests/id_only_record.h diff --git a/src/queries.cc b/src/queries.cc index 3e2e193..426d86d 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -217,16 +217,22 @@ InsertQuery::InsertQuery(sqlite3* db, const Reflection& record, void* p, bool au std::string InsertQuery::PrepareSql() const { std::string sql("INSERT INTO "); - sql += record_.name + " ("; + sql += record_.name; if (auto_increment_id_) { // Omit the id column (index 0) so SQLite assigns the next rowid itself auto columns = GetRecordColumnNames(); const std::vector columns_without_id(columns.begin() + 1, columns.end()); - sql += StringUtilities::Join(columns_without_id, ", ") + ") VALUES ("; - sql += Placeholders(columns_without_id.size()) + ");"; + if (columns_without_id.empty()) { + // The record has no columns besides id, so there is nothing to list; + // let SQLite assign the rowid and use defaults for every column + sql += " DEFAULT VALUES;"; + } else { + sql += " (" + StringUtilities::Join(columns_without_id, ", ") + ") VALUES ("; + sql += Placeholders(columns_without_id.size()) + ");"; + } } else { - sql += JoinedRecordColumnNames() + ") VALUES ("; + sql += " (" + JoinedRecordColumnNames() + ") VALUES ("; sql += Placeholders(record_.member_metadata.size()) + ");"; } diff --git a/tests/database_test.cc b/tests/database_test.cc index 18cf88e..1265875 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -25,6 +25,7 @@ #include #include "company.h" +#include "id_only_record.h" #include "person.h" #include "pet.h" @@ -140,6 +141,25 @@ TEST_F(DatabaseTest, AutoIdIsNotReusedAfterDeletingHighestRow) { EXPECT_EQ(3, third.id); } +TEST_F(DatabaseTest, AutoIncrementInsertForIdOnlyRecord) { + const auto& db = Database::Instance(); + + // A record whose only column is the implicit id must still insert via the + // auto-increment path (INSERT INTO ... DEFAULT VALUES) and get an assigned id + IdOnlyRecord first; + db.SaveAutoIncrement(first); + EXPECT_EQ(1, first.id); + + IdOnlyRecord second; + db.SaveAutoIncrement(second); + EXPECT_EQ(2, second.id); + + const auto all = db.FetchAll(); + EXPECT_EQ(2, all.size()); + EXPECT_EQ(1, all[0].id); + EXPECT_EQ(2, all[1].id); +} + TEST_F(DatabaseTest, MultipleInsertions) { const auto& db = Database::Instance(); diff --git a/tests/id_only_record.h b/tests/id_only_record.h new file mode 100644 index 0000000..289a8c7 --- /dev/null +++ b/tests/id_only_record.h @@ -0,0 +1,29 @@ +// 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 + +// A record with no user-defined fields besides the implicit id, used to exercise +// the auto-increment insert path for id-only tables (INSERT INTO ... DEFAULT VALUES) +#define REFLECTABLE IdOnlyRecord +#define FIELDS +#include "reflection.h" From c96bd6000ea023c44f449a911b389942ff3bca38 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 06:05:31 +0000 Subject: [PATCH 6/7] Avoid reading the omitted id binding in auto-increment inserts For auto-increment inserts the id column is omitted from the statement, yet Bindings() previously materialized every member via GetValues() and then erased the id binding. When the record's id was never initialized (e.g. an id-only record), this read an indeterminate int64_t before discarding it. GetValues() now accepts a start member index, and the auto-increment insert path builds bindings starting from the first non-id member, so the id value is never read. --- include/queries.h | 2 +- src/queries.cc | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/include/queries.h b/include/queries.h index 8605f50..92a9a94 100644 --- a/include/queries.h +++ b/include/queries.h @@ -71,7 +71,7 @@ class REFLECTION_EXPORT ExecutionQuery : public Query { protected: virtual std::vector Bindings() const; - std::vector GetValues(void* p) const; + std::vector GetValues(void* p, size_t start_member_index = 0) const; }; /// A query for which direct SQL prompts are used diff --git a/src/queries.cc b/src/queries.cc index 426d86d..4abd505 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -129,11 +129,11 @@ std::vector ExecutionQuery::Bindings() const { return {}; } -std::vector ExecutionQuery::GetValues(void* p) const { +std::vector ExecutionQuery::GetValues(void* p, size_t start_member_index) const { const auto& members = record_.member_metadata; std::vector values; - for (auto j = 0; j < members.size(); j++) { + for (auto j = start_member_index; j < members.size(); j++) { const auto current_storage_class = members[j].storage_class; SqlValue value; value.storage_class = current_storage_class; @@ -240,12 +240,9 @@ std::string InsertQuery::PrepareSql() const { } std::vector InsertQuery::Bindings() const { - auto values = GetValues(p_); - if (auto_increment_id_) { - // The id column is omitted from the statement, so drop its binding too - values.erase(values.begin()); - } - return values; + // For auto-increment inserts the id column is omitted from the statement, so skip the + // id member (index 0) entirely rather than reading its possibly-uninitialized value + return auto_increment_id_ ? GetValues(p_, 1) : GetValues(p_); } UpdateQuery::UpdateQuery(sqlite3* db, const Reflection& record, void* p) : ExecutionQuery(db, record), p_(p) {} From 859b44470bcb246e7d5d9308beadcf8054c5ddbd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 07:16:37 +0000 Subject: [PATCH 7/7] Use a skip_id flag instead of a start index in GetValues The auto-increment insert path only ever needs to skip the leading id member, so a boolean skip_id expresses the intent more clearly than a numeric start index at the call site. --- include/queries.h | 2 +- src/queries.cc | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/queries.h b/include/queries.h index 92a9a94..453b1b5 100644 --- a/include/queries.h +++ b/include/queries.h @@ -71,7 +71,7 @@ class REFLECTION_EXPORT ExecutionQuery : public Query { protected: virtual std::vector Bindings() const; - std::vector GetValues(void* p, size_t start_member_index = 0) const; + std::vector GetValues(void* p, bool skip_id = false) const; }; /// A query for which direct SQL prompts are used diff --git a/src/queries.cc b/src/queries.cc index 4abd505..092de2b 100644 --- a/src/queries.cc +++ b/src/queries.cc @@ -129,11 +129,12 @@ std::vector ExecutionQuery::Bindings() const { return {}; } -std::vector ExecutionQuery::GetValues(void* p, size_t start_member_index) const { +std::vector ExecutionQuery::GetValues(void* p, bool skip_id) const { const auto& members = record_.member_metadata; std::vector values; - for (auto j = start_member_index; j < members.size(); j++) { + // The id is always the first member (index 0); skip it when the caller asks + for (size_t j = skip_id ? 1 : 0; j < members.size(); j++) { const auto current_storage_class = members[j].storage_class; SqlValue value; value.storage_class = current_storage_class; @@ -241,8 +242,8 @@ std::string InsertQuery::PrepareSql() const { std::vector InsertQuery::Bindings() const { // For auto-increment inserts the id column is omitted from the statement, so skip the - // id member (index 0) entirely rather than reading its possibly-uninitialized value - return auto_increment_id_ ? GetValues(p_, 1) : GetValues(p_); + // id member entirely rather than reading its possibly-uninitialized value + return GetValues(p_, auto_increment_id_); } UpdateQuery::UpdateQuery(sqlite3* db, const Reflection& record, void* p) : ExecutionQuery(db, record), p_(p) {}