Fix SaveAutoIncrement to write back a database-assigned, non-reused id (#6)

.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",

README.md

@@ -182,18 +182,22 @@ 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 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};
 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<Person> 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

include/database.h

@@ -107,28 +107,42 @@ class REFLECTION_EXPORT Database {
     /// This corresponds to an INSERT query in the SQL syntax
     template <typename T>
     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 <typename T>
-    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 <typename T>
     void Save(const std::vector<T>& 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 <typename T>
-    void SaveAutoIncrement(const std::vector<T>& models) const {
-        Save(models, true);
+    void SaveAutoIncrement(std::vector<T>& 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 <typename T>
-    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<T>();
-            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 <typename T>
-    void Save(const std::vector<T>& 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<T>() : 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;
 

include/queries.h

@@ -71,7 +71,7 @@ class REFLECTION_EXPORT ExecutionQuery : public Query {
 
 protected:
     virtual std::vector<SqlValue> Bindings() const;
-    std::vector<SqlValue> GetValues(void* p) const;
+    std::vector<SqlValue> GetValues(void* p, bool skip_id = false) const;
 };
 
 /// A query for which direct SQL prompts are used
@@ -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<SqlValue> 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

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();

src/queries.cc

@@ -129,11 +129,12 @@ std::vector<SqlValue> ExecutionQuery::Bindings() const {
     return {};
 }
 
-std::vector<SqlValue> ExecutionQuery::GetValues(void* p) const {
+std::vector<SqlValue> ExecutionQuery::GetValues(void* p, bool skip_id) const {
     const auto& members = record_.member_metadata;
     std::vector<SqlValue> values;
 
-    for (auto j = 0; 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;
@@ -194,7 +195,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)
@@ -210,17 +213,37 @@ std::vector<SqlValue> 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<std::string> columns_without_id(columns.begin() + 1, columns.end());
+        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 += Placeholders(record_.member_metadata.size()) + ");";
+    }
+
     return sql;
 }
 
 std::vector<SqlValue> InsertQuery::Bindings() const {
-    return GetValues(p_);
+    // For auto-increment inserts the id column is omitted from the statement, so skip the
+    // 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) {}

tests/database_test.cc

@@ -25,6 +25,7 @@
 #include <gtest/gtest.h>
 
 #include "company.h"
+#include "id_only_record.h"
 #include "person.h"
 #include "pet.h"
 
@@ -68,9 +69,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<Person>();
     EXPECT_EQ(1, all_persons.size());
 
@@ -80,6 +84,82 @@ TEST_F(DatabaseTest, SingleInsertionWithAutoIdIncrement) {
     EXPECT_EQ(1, all_persons[0].id);
 }
 
+TEST_F(DatabaseTest, MultipleInsertionsWithAutoIdIncrement) {
+    const auto& db = Database::Instance();
+
+    std::vector<Person> 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<Person>();
+    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, 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, 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<IdOnlyRecord>();
+    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();
 

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"