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", diff --git a/README.md b/README.md index 4a1fb50..089941f 100644 --- a/README.md +++ b/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 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..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) const; + std::vector 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 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..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) const { +std::vector ExecutionQuery::GetValues(void* p, bool skip_id) const { const auto& members = record_.member_metadata; std::vector 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 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()); + 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 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) {} diff --git a/tests/database_test.cc b/tests/database_test.cc index cf97389..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" @@ -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(); 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 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, 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(); + 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"