From f286d95b80364b828911f9d06ae6ff7ddfbba40a Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sun, 2 Feb 2025 21:58:00 +0200 Subject: [PATCH 01/18] Corrected error handling for errors when opening the DB --- dev/connection_holder.h | 2 +- include/sqlite_orm/sqlite_orm.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 11e6dd0a7..777fb72d9 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -18,7 +18,7 @@ namespace sqlite_orm { if (1 == ++this->_retain_count) { auto rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + throw_translated_sqlite_error(rc); } } } diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index cbd8695b6..5d09a469b 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13660,7 +13660,7 @@ namespace sqlite_orm { if (1 == ++this->_retain_count) { auto rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + throw_translated_sqlite_error(rc); } } } From b8a832b7ae94a70739ae7e0f872546fc02d8e920 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sun, 2 Feb 2025 22:03:52 +0200 Subject: [PATCH 02/18] Threadsafe database connection holder The connection holder should be performant in all variants: 1. single-threaded use 2. opened once (open forever) 3. concurrent open/close Hence, a light-weight binary semaphore is used to synchronize opening and closing a database connection. --- dev/connection_holder.h | 114 ++++++++++++++++++++++++++++-- dev/functional/config.h | 4 ++ include/sqlite_orm/sqlite_orm.h | 118 ++++++++++++++++++++++++++++++-- 3 files changed, 224 insertions(+), 12 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 777fb72d9..f56c776e3 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -2,20 +2,114 @@ #include #include +#ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED +#include +#endif #include // std::string #include "error_code.h" namespace sqlite_orm { - namespace internal { +#ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED + /** + * The connection holder should be performant in all variants: + 1. single-threaded use + 2. opened once (open forever) + 3. concurrent open/close + + Hence, a light-weight binary semaphore is used to synchronize opening and closing a database connection. + */ struct connection_holder { + struct maybe_lock { + maybe_lock(std::binary_semaphore& sync, bool shouldLock) noexcept(noexcept(sync.acquire())) : + isSynced{shouldLock}, sync{sync} { + if (shouldLock) { + sync.acquire(); + } + } + + ~maybe_lock() { + if (isSynced) { + sync.release(); + } + } + + const bool isSynced; + std::binary_semaphore& sync; + }; + + explicit connection_holder(std::string filename) : filename(std::move(filename)) {} + + void retain() { + const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + + // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter + // `!maybeLock.isSynced`: we presume that this the connection is opened once in a single-threaded context [open forever]. + // so we can just use an atomic operation but don't need sequencing due to `prevCount > 0`. + const int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); + + if (prevCount == 0 || (maybeLock.isSynced && !this->db)) { + if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) + [[unlikely]] /*unexpected*/ { + throw_translated_sqlite_error(this->db); + } + } + } + + void release() { + const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + + if (int prevCount = this->_retain_count.fetch_sub( + 1, + maybeLock.isSynced + // the lock above already synchronized everything, so we can just atomically decrement the counter + ? std::memory_order_relaxed + // the counter must serve as a synchronization point + : std::memory_order_acq_rel); + prevCount > 1) { + return; + } + + if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; + } + } + + sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` + return this->db; + } + + /** + * @attention While retrieving the reference count value is well-defined it makes only sense at single-threaded points in code + */ + int retain_count() const { + return this->_retain_count.load(std::memory_order_relaxed); + } + + const std::string filename; - connection_holder(std::string filename_) : filename(std::move(filename_)) {} + protected: + sqlite3* db = nullptr; + + private: + std::binary_semaphore _sync{1}; + const bool _openedForeverHint = false; + std::atomic_int _retain_count{}; + }; +#else + struct connection_holder { + explicit connection_holder(std::string filename) : filename(std::move(filename)) {} void retain() { - if (1 == ++this->_retain_count) { + // first one opens the connection. + // we presume that this the connection is opened once in a single-threaded context [also open forever]. + // so we can just use an atomic read but don't need sequencing due to `prevCount > 0`. + if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { auto rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { throw_translated_sqlite_error(rc); @@ -24,28 +118,36 @@ namespace sqlite_orm { } void release() { - if (0 == --this->_retain_count) { + // last one closes the connection. + // we assume that this might happen by any thread. + if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { auto rc = sqlite3_close(this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; } } } sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` return this->db; } int retain_count() const { - return this->_retain_count; + return this->_retain_count.load(std::memory_order_relaxed); } const std::string filename; protected: sqlite3* db = nullptr; + + private: std::atomic_int _retain_count{}; }; +#endif struct connection_ref { connection_ref(connection_holder& holder) : holder(&holder) { diff --git a/dev/functional/config.h b/dev/functional/config.h index 4b98290eb..6cf9736a6 100644 --- a/dev/functional/config.h +++ b/dev/functional/config.h @@ -58,6 +58,10 @@ #define SQLITE_ORM_CPP20_RANGES_SUPPORTED #endif +#if __cpp_lib_semaphore >= 201907L +#define SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED +#endif + // C++20 or later (unfortunately there's no feature test macro). // Stupidly, clang says C++20, but `std::default_sentinel_t` was only implemented in libc++ 13 and libstd++-v3 10 // (the latter is used on Linux). diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 5d09a469b..75a382b66 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -257,6 +257,10 @@ using std::nullptr_t; #define SQLITE_ORM_CPP20_RANGES_SUPPORTED #endif +#if __cpp_lib_semaphore >= 201907L +#define SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED +#endif + // C++20 or later (unfortunately there's no feature test macro). // Stupidly, clang says C++20, but `std::default_sentinel_t` was only implemented in libc++ 13 and libstd++-v3 10 // (the latter is used on Linux). @@ -13644,20 +13648,114 @@ namespace sqlite_orm { #include #include +#ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED +#include +#endif #include // std::string // #include "error_code.h" namespace sqlite_orm { - namespace internal { +#ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED + /** + * The connection holder should be performant in all variants: + 1. single-threaded use + 2. opened once (open forever) + 3. concurrent open/close + + Hence, a light-weight binary semaphore is used to synchronize opening and closing a database connection. + */ struct connection_holder { + struct maybe_lock { + maybe_lock(std::binary_semaphore& sync, bool shouldLock) noexcept(noexcept(sync.acquire())) : + isSynced{shouldLock}, sync{sync} { + if (shouldLock) { + sync.acquire(); + } + } - connection_holder(std::string filename_) : filename(std::move(filename_)) {} + ~maybe_lock() { + if (isSynced) { + sync.release(); + } + } + + const bool isSynced; + std::binary_semaphore& sync; + }; + + explicit connection_holder(std::string filename) : filename(std::move(filename)) {} void retain() { - if (1 == ++this->_retain_count) { + const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + + // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter + // `!maybeLock.isSynced`: we presume that this the connection is opened once in a single-threaded context [open forever]. + // so we can just use an atomic operation but don't need sequencing due to `prevCount > 0`. + const int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); + + if (prevCount == 0 || (maybeLock.isSynced && !this->db)) { + if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) + [[unlikely]] /*unexpected*/ { + throw_translated_sqlite_error(this->db); + } + } + } + + void release() { + const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + + if (int prevCount = this->_retain_count.fetch_sub( + 1, + maybeLock.isSynced + // the lock above already synchronized everything, so we can just atomically decrement the counter + ? std::memory_order_relaxed + // the counter must serve as a synchronization point + : std::memory_order_acq_rel); + prevCount > 1) { + return; + } + + if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; + } + } + + sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` + return this->db; + } + + /** + * @attention While retrieving the reference count value is well-defined it makes only sense at single-threaded points in code + */ + int retain_count() const { + return this->_retain_count.load(std::memory_order_relaxed); + } + + const std::string filename; + + protected: + sqlite3* db = nullptr; + + private: + std::binary_semaphore _sync{1}; + const bool _openedForeverHint = false; + std::atomic_int _retain_count{}; + }; +#else + struct connection_holder { + explicit connection_holder(std::string filename) : filename(std::move(filename)) {} + + void retain() { + // first one opens the connection. + // we presume that this the connection is opened once in a single-threaded context [also open forever]. + // so we can just use an atomic read but don't need sequencing due to `prevCount > 0`. + if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { auto rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { throw_translated_sqlite_error(rc); @@ -13666,28 +13764,36 @@ namespace sqlite_orm { } void release() { - if (0 == --this->_retain_count) { + // last one closes the connection. + // we assume that this might happen by any thread. + if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { auto rc = sqlite3_close(this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(db); + throw_translated_sqlite_error(this->db); + } else { + this->db = nullptr; } } } sqlite3* get() const { + // note: ensuring a valid DB handle was already memory ordered with `retain()` return this->db; } int retain_count() const { - return this->_retain_count; + return this->_retain_count.load(std::memory_order_relaxed); } const std::string filename; protected: sqlite3* db = nullptr; + + private: std::atomic_int _retain_count{}; }; +#endif struct connection_ref { connection_ref(connection_holder& holder) : holder(&holder) { From 8285dab30e99cb0599d820c2f6a94384df33a172 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Wed, 5 Feb 2025 14:18:59 +0200 Subject: [PATCH 03/18] Simplified logic of connection holders 'retain' function --- dev/connection_holder.h | 28 ++++++++++++++++------------ include/sqlite_orm/sqlite_orm.h | 28 ++++++++++++++++------------ 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index f56c776e3..453a63af4 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -46,15 +46,17 @@ namespace sqlite_orm { const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter - // `!maybeLock.isSynced`: we presume that this the connection is opened once in a single-threaded context [open forever]. - // so we can just use an atomic operation but don't need sequencing due to `prevCount > 0`. - const int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); + // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [open forever]. + // so we can just use an atomic increment but don't need sequencing due to always `prevCount > 0`. + if (int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + return; + } - if (prevCount == 0 || (maybeLock.isSynced && !this->db)) { - if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) - [[unlikely]] /*unexpected*/ { - throw_translated_sqlite_error(this->db); - } + // first one opens the connection. + + if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) + [[unlikely]] /*possible, but unexpected*/ { + throw_translated_sqlite_error(this->db); } } @@ -72,6 +74,8 @@ namespace sqlite_orm { return; } + // last one closes the connection. + if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { throw_translated_sqlite_error(this->db); } else { @@ -108,11 +112,11 @@ namespace sqlite_orm { void retain() { // first one opens the connection. // we presume that this the connection is opened once in a single-threaded context [also open forever]. - // so we can just use an atomic read but don't need sequencing due to `prevCount > 0`. + // so we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { - auto rc = sqlite3_open(this->filename.c_str(), &this->db); + int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(rc); + throw_translated_sqlite_error(this->db); } } } @@ -121,7 +125,7 @@ namespace sqlite_orm { // last one closes the connection. // we assume that this might happen by any thread. if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { - auto rc = sqlite3_close(this->db); + int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) { throw_translated_sqlite_error(this->db); } else { diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 75a382b66..27d4e2180 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13692,15 +13692,17 @@ namespace sqlite_orm { const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter - // `!maybeLock.isSynced`: we presume that this the connection is opened once in a single-threaded context [open forever]. - // so we can just use an atomic operation but don't need sequencing due to `prevCount > 0`. - const int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); + // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [open forever]. + // so we can just use an atomic increment but don't need sequencing due to always `prevCount > 0`. + if (int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + return; + } - if (prevCount == 0 || (maybeLock.isSynced && !this->db)) { - if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) - [[unlikely]] /*unexpected*/ { - throw_translated_sqlite_error(this->db); - } + // first one opens the connection. + + if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) + [[unlikely]] /*possible, but unexpected*/ { + throw_translated_sqlite_error(this->db); } } @@ -13718,6 +13720,8 @@ namespace sqlite_orm { return; } + // last one closes the connection. + if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { throw_translated_sqlite_error(this->db); } else { @@ -13754,11 +13758,11 @@ namespace sqlite_orm { void retain() { // first one opens the connection. // we presume that this the connection is opened once in a single-threaded context [also open forever]. - // so we can just use an atomic read but don't need sequencing due to `prevCount > 0`. + // so we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { - auto rc = sqlite3_open(this->filename.c_str(), &this->db); + int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) { - throw_translated_sqlite_error(rc); + throw_translated_sqlite_error(this->db); } } } @@ -13767,7 +13771,7 @@ namespace sqlite_orm { // last one closes the connection. // we assume that this might happen by any thread. if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { - auto rc = sqlite3_close(this->db); + int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) { throw_translated_sqlite_error(this->db); } else { From 458ec11d28d01d673ae93cb85dabd48ecf8f407a Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Fri, 7 Feb 2025 18:59:31 +0200 Subject: [PATCH 04/18] Set up the database under the same synchronization lock as opening it --- dev/connection_holder.h | 34 ++++++++++++++-- dev/pragma.h | 9 +++-- dev/storage_base.h | 26 +++++-------- include/sqlite_orm/sqlite_orm.h | 69 +++++++++++++++++++++------------ 4 files changed, 90 insertions(+), 48 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index a5e0286d7..d1ef7fb78 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -6,6 +6,7 @@ #ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED #include #endif +#include // std::function #include // std::string #endif @@ -42,7 +43,15 @@ namespace sqlite_orm { std::binary_semaphore& sync; }; - connection_holder(std::string filename) : filename(std::move(filename)) {} + connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : + filename(std::move(filename)), _openedForeverHint{openedForeverHint}, + _onAfterOpen{std::move(onAfterOpen)} {} + + connection_holder(const connection_holder&) = delete; + + connection_holder(const connection_holder& other, std::function onAfterOpen) : + filename{other.filename}, _openedForeverHint{other._openedForeverHint}, + _onAfterOpen{std::move(onAfterOpen)} {} void retain() { const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; @@ -54,12 +63,16 @@ namespace sqlite_orm { return; } - // first one opens the connection. + // first one opens and sets up the connection. if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } + + if (this->_onAfterOpen) { + this->_onAfterOpen(this->db); + } } void release() { @@ -103,13 +116,21 @@ namespace sqlite_orm { sqlite3* db = nullptr; private: - std::binary_semaphore _sync{1}; const bool _openedForeverHint = false; + const std::function _onAfterOpen; + std::binary_semaphore _sync{1}; std::atomic_int _retain_count{}; }; #else struct connection_holder { - connection_holder(std::string filename) : filename(std::move(filename)) {} + connection_holder(std::string filename, + bool /*openedForeverHint*/, + std::function onAfterOpen) : + filename(std::move(filename)), _onAfterOpen{std::move(onAfterOpen)} {} + + connection_holder(const connection_holder&) = delete; + connection_holder(const connection_holder& other, std::function onAfterOpen) : + filename{other.filename}, _onAfterOpen{std::move(onAfterOpen)} {} void retain() { // first one opens the connection. @@ -121,6 +142,10 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } } + + if (this->_onAfterOpen) { + this->_onAfterOpen(this->db); + } } void release() { @@ -154,6 +179,7 @@ namespace sqlite_orm { sqlite3* db = nullptr; private: + const std::function _onAfterOpen; std::atomic_int _retain_count{}; }; #endif diff --git a/dev/pragma.h b/dev/pragma.h index d1868fae2..aef4dc9e4 100644 --- a/dev/pragma.h +++ b/dev/pragma.h @@ -258,11 +258,12 @@ namespace sqlite_orm { } void set_pragma_impl(const std::string& query, sqlite3* db = nullptr) { - auto con = this->get_connection(); - if (db == nullptr) { - db = con.get(); + if (db) { + perform_void_exec(db, query); + } else { + auto con = this->get_connection(); + perform_void_exec(con.get(), query); } - perform_void_exec(db, query); } }; } diff --git a/dev/storage_base.h b/dev/storage_base.h index cfc0638ad..da4a6be82 100644 --- a/dev/storage_base.h +++ b/dev/storage_base.h @@ -295,9 +295,6 @@ namespace sqlite_orm { void open_forever() { this->isOpenedForever = true; this->connection->retain(); - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } } /** @@ -610,7 +607,7 @@ namespace sqlite_orm { } backup_t make_backup_to(const std::string& filename) { - auto holder = std::make_unique(filename); + auto holder = std::make_unique(filename, true, nullptr); connection_ref conRef{*holder}; return {conRef, "main", this->get_connection(), "main", std::move(holder)}; } @@ -620,7 +617,7 @@ namespace sqlite_orm { } backup_t make_backup_from(const std::string& filename) { - auto holder = std::make_unique(filename); + auto holder = std::make_unique(filename, true, nullptr); connection_ref conRef{*holder}; return {this->get_connection(), "main", conRef, "main", std::move(holder)}; } @@ -668,22 +665,25 @@ namespace sqlite_orm { pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), - connection(std::make_unique(std::move(filename))), + connection(std::make_unique( + std::move(filename), + inMemory, + std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(foreignKeysCount) { if (this->inMemory) { this->connection->retain(); - this->on_open_internal(this->connection->get()); } } storage_base(const storage_base& other) : on_open(other.on_open), pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(other.inMemory), - connection(std::make_unique(other.connection->filename)), + connection(std::make_unique( + *other.connection, + std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(other.cachedForeignKeysCount) { if (this->inMemory) { this->connection->retain(); - this->on_open_internal(this->connection->get()); } } @@ -698,18 +698,12 @@ namespace sqlite_orm { void begin_transaction_internal(const std::string& query) { this->connection->retain(); - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } sqlite3* db = this->connection->get(); perform_void_exec(db, query); } connection_ref get_connection() { connection_ref res{*this->connection}; - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } return res; } @@ -735,7 +729,7 @@ namespace sqlite_orm { } #endif if (this->pragma.synchronous_ != -1) { - this->pragma.synchronous(this->pragma.synchronous_); + this->pragma.set_pragma("synchronous", this->pragma.synchronous_, db); } if (this->pragma.journal_mode_ != -1) { diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 4eea355e9..80dc81422 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13883,6 +13883,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_CPP20_SEMAPHORE_SUPPORTED #include #endif +#include // std::function #include // std::string #endif @@ -13919,7 +13920,15 @@ namespace sqlite_orm { std::binary_semaphore& sync; }; - connection_holder(std::string filename) : filename(std::move(filename)) {} + connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : + filename(std::move(filename)), _openedForeverHint{openedForeverHint}, + _onAfterOpen{std::move(onAfterOpen)} {} + + connection_holder(const connection_holder&) = delete; + + connection_holder(const connection_holder& other, std::function onAfterOpen) : + filename{other.filename}, _openedForeverHint{other._openedForeverHint}, + _onAfterOpen{std::move(onAfterOpen)} {} void retain() { const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; @@ -13931,12 +13940,16 @@ namespace sqlite_orm { return; } - // first one opens the connection. + // first one opens and sets up the connection. if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } + + if (this->_onAfterOpen) { + this->_onAfterOpen(this->db); + } } void release() { @@ -13980,13 +13993,21 @@ namespace sqlite_orm { sqlite3* db = nullptr; private: - std::binary_semaphore _sync{1}; const bool _openedForeverHint = false; + const std::function _onAfterOpen; + std::binary_semaphore _sync{1}; std::atomic_int _retain_count{}; }; #else struct connection_holder { - connection_holder(std::string filename) : filename(std::move(filename)) {} + connection_holder(std::string filename, + bool /*openedForeverHint*/, + std::function onAfterOpen) : + filename(std::move(filename)), _onAfterOpen{std::move(onAfterOpen)} {} + + connection_holder(const connection_holder&) = delete; + connection_holder(const connection_holder& other, std::function onAfterOpen) : + filename{other.filename}, _onAfterOpen{std::move(onAfterOpen)} {} void retain() { // first one opens the connection. @@ -13998,6 +14019,10 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } } + + if (this->_onAfterOpen) { + this->_onAfterOpen(this->db); + } } void release() { @@ -14031,6 +14056,7 @@ namespace sqlite_orm { sqlite3* db = nullptr; private: + const std::function _onAfterOpen; std::atomic_int _retain_count{}; }; #endif @@ -17051,11 +17077,12 @@ namespace sqlite_orm { } void set_pragma_impl(const std::string& query, sqlite3* db = nullptr) { - auto con = this->get_connection(); - if (db == nullptr) { - db = con.get(); + if (db) { + perform_void_exec(db, query); + } else { + auto con = this->get_connection(); + perform_void_exec(con.get(), query); } - perform_void_exec(db, query); } }; } @@ -18069,9 +18096,6 @@ namespace sqlite_orm { void open_forever() { this->isOpenedForever = true; this->connection->retain(); - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } } /** @@ -18384,7 +18408,7 @@ namespace sqlite_orm { } backup_t make_backup_to(const std::string& filename) { - auto holder = std::make_unique(filename); + auto holder = std::make_unique(filename, true, nullptr); connection_ref conRef{*holder}; return {conRef, "main", this->get_connection(), "main", std::move(holder)}; } @@ -18394,7 +18418,7 @@ namespace sqlite_orm { } backup_t make_backup_from(const std::string& filename) { - auto holder = std::make_unique(filename); + auto holder = std::make_unique(filename, true, nullptr); connection_ref conRef{*holder}; return {this->get_connection(), "main", conRef, "main", std::move(holder)}; } @@ -18442,22 +18466,25 @@ namespace sqlite_orm { pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), - connection(std::make_unique(std::move(filename))), + connection(std::make_unique( + std::move(filename), + inMemory, + std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(foreignKeysCount) { if (this->inMemory) { this->connection->retain(); - this->on_open_internal(this->connection->get()); } } storage_base(const storage_base& other) : on_open(other.on_open), pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(other.inMemory), - connection(std::make_unique(other.connection->filename)), + connection(std::make_unique( + *other.connection, + std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(other.cachedForeignKeysCount) { if (this->inMemory) { this->connection->retain(); - this->on_open_internal(this->connection->get()); } } @@ -18472,18 +18499,12 @@ namespace sqlite_orm { void begin_transaction_internal(const std::string& query) { this->connection->retain(); - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } sqlite3* db = this->connection->get(); perform_void_exec(db, query); } connection_ref get_connection() { connection_ref res{*this->connection}; - if (1 == this->connection->retain_count()) { - this->on_open_internal(this->connection->get()); - } return res; } @@ -18509,7 +18530,7 @@ namespace sqlite_orm { } #endif if (this->pragma.synchronous_ != -1) { - this->pragma.synchronous(this->pragma.synchronous_); + this->pragma.set_pragma("synchronous", this->pragma.synchronous_, db); } if (this->pragma.journal_mode_ != -1) { From cbb81ac0e01e09e9e850e975e76f871129e08025 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Fri, 7 Feb 2025 19:02:07 +0200 Subject: [PATCH 05/18] Unqualified use of member variables beginning with an underscore --- dev/connection_holder.h | 24 ++++++++++++------------ include/sqlite_orm/sqlite_orm.h | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index d1ef7fb78..9c09503b2 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -54,12 +54,12 @@ namespace sqlite_orm { _onAfterOpen{std::move(onAfterOpen)} {} void retain() { - const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + const maybe_lock maybeLock{_sync, !_openedForeverHint}; // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + if (int prevCount = _retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { return; } @@ -70,15 +70,15 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } - if (this->_onAfterOpen) { - this->_onAfterOpen(this->db); + if (_onAfterOpen) { + _onAfterOpen(this->db); } } void release() { - const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + const maybe_lock maybeLock{_sync, !_openedForeverHint}; - if (int prevCount = this->_retain_count.fetch_sub( + if (int prevCount = _retain_count.fetch_sub( 1, maybeLock.isSynced // the lock above already synchronized everything, so we can just atomically decrement the counter @@ -107,7 +107,7 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return this->_retain_count.load(std::memory_order_relaxed); + return _retain_count.load(std::memory_order_relaxed); } const std::string filename; @@ -136,22 +136,22 @@ namespace sqlite_orm { // first one opens the connection. // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + if (_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } } - if (this->_onAfterOpen) { - this->_onAfterOpen(this->db); + if (_onAfterOpen) { + _onAfterOpen(this->db); } } void release() { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. - if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); @@ -170,7 +170,7 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return this->_retain_count.load(std::memory_order_relaxed); + return _retain_count.load(std::memory_order_relaxed); } const std::string filename; diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 80dc81422..9e401d671 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13931,12 +13931,12 @@ namespace sqlite_orm { _onAfterOpen{std::move(onAfterOpen)} {} void retain() { - const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + const maybe_lock maybeLock{_sync, !_openedForeverHint}; // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (int prevCount = this->_retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + if (int prevCount = _retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { return; } @@ -13947,15 +13947,15 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } - if (this->_onAfterOpen) { - this->_onAfterOpen(this->db); + if (_onAfterOpen) { + _onAfterOpen(this->db); } } void release() { - const maybe_lock maybeLock{this->_sync, !this->_openedForeverHint}; + const maybe_lock maybeLock{_sync, !_openedForeverHint}; - if (int prevCount = this->_retain_count.fetch_sub( + if (int prevCount = _retain_count.fetch_sub( 1, maybeLock.isSynced // the lock above already synchronized everything, so we can just atomically decrement the counter @@ -13984,7 +13984,7 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return this->_retain_count.load(std::memory_order_relaxed); + return _retain_count.load(std::memory_order_relaxed); } const std::string filename; @@ -14013,22 +14013,22 @@ namespace sqlite_orm { // first one opens the connection. // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (this->_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + if (_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } } - if (this->_onAfterOpen) { - this->_onAfterOpen(this->db); + if (_onAfterOpen) { + _onAfterOpen(this->db); } } void release() { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. - if (this->_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); @@ -14047,7 +14047,7 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return this->_retain_count.load(std::memory_order_relaxed); + return _retain_count.load(std::memory_order_relaxed); } const std::string filename; From 23f66e0a2604caa4e33ee25107e27e268a7caa4e Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Mon, 10 Feb 2025 20:40:27 +0200 Subject: [PATCH 06/18] Reordered connection holder's members for the purpose of L1 cache line --- dev/connection_holder.h | 55 +++++++++++++------- dev/functional/cxx_core_features.h | 5 +- dev/functional/cxx_new.h | 23 +++++++++ include/sqlite_orm/sqlite_orm.h | 83 ++++++++++++++++++++++-------- 4 files changed, 124 insertions(+), 42 deletions(-) create mode 100644 dev/functional/cxx_new.h diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 9c09503b2..68d8621d0 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -10,6 +10,7 @@ #include // std::string #endif +#include "functional/cxx_new.h" #include "error_code.h" namespace sqlite_orm { @@ -44,8 +45,8 @@ namespace sqlite_orm { }; connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : - filename(std::move(filename)), _openedForeverHint{openedForeverHint}, - _onAfterOpen{std::move(onAfterOpen)} {} + _openedForeverHint{openedForeverHint}, _onAfterOpen{std::move(onAfterOpen)}, + filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; @@ -59,7 +60,7 @@ namespace sqlite_orm { // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (int prevCount = _retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + if (int prevCount = _retainCount.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { return; } @@ -78,7 +79,7 @@ namespace sqlite_orm { void release() { const maybe_lock maybeLock{_sync, !_openedForeverHint}; - if (int prevCount = _retain_count.fetch_sub( + if (int prevCount = _retainCount.fetch_sub( 1, maybeLock.isSynced // the lock above already synchronized everything, so we can just atomically decrement the counter @@ -107,36 +108,41 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return _retain_count.load(std::memory_order_relaxed); + return _retainCount.load(std::memory_order_relaxed); } - const std::string filename; - protected: - sqlite3* db = nullptr; + alignas(polyfill::hardware_destructive_interference_size) sqlite3* db = nullptr; private: + std::atomic_int _retainCount{}; const bool _openedForeverHint = false; - const std::function _onAfterOpen; std::binary_semaphore _sync{1}; - std::atomic_int _retain_count{}; + + private: + alignas( + polyfill::hardware_destructive_interference_size) const std::function _onAfterOpen; + + public: + const std::string filename; }; #else struct connection_holder { connection_holder(std::string filename, bool /*openedForeverHint*/, std::function onAfterOpen) : - filename(std::move(filename)), _onAfterOpen{std::move(onAfterOpen)} {} + _onAfterOpen{std::move(onAfterOpen)}, filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; + connection_holder(const connection_holder& other, std::function onAfterOpen) : - filename{other.filename}, _onAfterOpen{std::move(onAfterOpen)} {} + _onAfterOpen{std::move(onAfterOpen)}, filename{other.filename} {} void retain() { // first one opens the connection. // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); @@ -151,7 +157,7 @@ namespace sqlite_orm { void release() { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. - if (_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (_retainCount.fetch_sub(1, std::memory_order_acq_rel) == 1) { int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); @@ -170,17 +176,26 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return _retain_count.load(std::memory_order_relaxed); + return _retainCount.load(std::memory_order_relaxed); } - const std::string filename; - protected: - sqlite3* db = nullptr; +#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED + alignas(polyfill::hardware_destructive_interference_size) +#endif + sqlite3* db = nullptr; + + private: + std::atomic_int _retainCount{}; private: - const std::function _onAfterOpen; - std::atomic_int _retain_count{}; +#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED + alignas(polyfill::hardware_destructive_interference_size) +#endif + const std::function _onAfterOpen; + + public: + const std::string filename; }; #endif diff --git a/dev/functional/cxx_core_features.h b/dev/functional/cxx_core_features.h index 93977a4f2..909698217 100644 --- a/dev/functional/cxx_core_features.h +++ b/dev/functional/cxx_core_features.h @@ -49,9 +49,12 @@ #define SQLITE_ORM_STRUCTURED_BINDINGS_SUPPORTED #endif +#if __cpp_aligned_new >= 201606L +#define SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#endif + #if __cpp_generic_lambdas >= 201707L #define SQLITE_ORM_EXPLICIT_GENERIC_LAMBDA_SUPPORTED -#else #endif #if __cpp_init_captures >= 201803L diff --git a/dev/functional/cxx_new.h b/dev/functional/cxx_new.h new file mode 100644 index 000000000..c360d9c92 --- /dev/null +++ b/dev/functional/cxx_new.h @@ -0,0 +1,23 @@ +#pragma once + +#ifdef SQLITE_ORM_IMPORT_STD_MODULE +#include +#else +#include +#endif + +namespace sqlite_orm { + namespace internal { + namespace polyfill { +#if __cpp_lib_hardware_interference_size >= 201703L + using std::hardware_constructive_interference_size; + using std::hardware_destructive_interference_size; +#else + constexpr size_t hardware_constructive_interference_size = 64; + constexpr size_t hardware_destructive_interference_size = 64; +#endif + } + } + + namespace polyfill = internal::polyfill; +} diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 9e401d671..a88693222 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -98,9 +98,12 @@ using std::nullptr_t; #define SQLITE_ORM_STRUCTURED_BINDINGS_SUPPORTED #endif +#if __cpp_aligned_new >= 201606L +#define SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#endif + #if __cpp_generic_lambdas >= 201707L #define SQLITE_ORM_EXPLICIT_GENERIC_LAMBDA_SUPPORTED -#else #endif #if __cpp_init_captures >= 201803L @@ -13887,6 +13890,30 @@ namespace sqlite_orm { #include // std::string #endif +// #include "functional/cxx_new.h" + +#ifdef SQLITE_ORM_IMPORT_STD_MODULE +#include +#else +#include +#endif + +namespace sqlite_orm { + namespace internal { + namespace polyfill { +#if __cpp_lib_hardware_interference_size >= 201703L + using std::hardware_constructive_interference_size; + using std::hardware_destructive_interference_size; +#else + constexpr size_t hardware_constructive_interference_size = 64; + constexpr size_t hardware_destructive_interference_size = 64; +#endif + } + } + + namespace polyfill = internal::polyfill; +} + // #include "error_code.h" namespace sqlite_orm { @@ -13921,8 +13948,8 @@ namespace sqlite_orm { }; connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : - filename(std::move(filename)), _openedForeverHint{openedForeverHint}, - _onAfterOpen{std::move(onAfterOpen)} {} + _openedForeverHint{openedForeverHint}, _onAfterOpen{std::move(onAfterOpen)}, + filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; @@ -13936,7 +13963,7 @@ namespace sqlite_orm { // `maybeLock.isSynced`: the lock above already synchronized everything, so we can just atomically increment the counter // `!maybeLock.isSynced`: we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (int prevCount = _retain_count.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { + if (int prevCount = _retainCount.fetch_add(1, std::memory_order_relaxed); prevCount > 0) { return; } @@ -13955,7 +13982,7 @@ namespace sqlite_orm { void release() { const maybe_lock maybeLock{_sync, !_openedForeverHint}; - if (int prevCount = _retain_count.fetch_sub( + if (int prevCount = _retainCount.fetch_sub( 1, maybeLock.isSynced // the lock above already synchronized everything, so we can just atomically decrement the counter @@ -13984,36 +14011,41 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return _retain_count.load(std::memory_order_relaxed); + return _retainCount.load(std::memory_order_relaxed); } - const std::string filename; - protected: - sqlite3* db = nullptr; + alignas(polyfill::hardware_destructive_interference_size) sqlite3* db = nullptr; private: + std::atomic_int _retainCount{}; const bool _openedForeverHint = false; - const std::function _onAfterOpen; std::binary_semaphore _sync{1}; - std::atomic_int _retain_count{}; + + private: + alignas( + polyfill::hardware_destructive_interference_size) const std::function _onAfterOpen; + + public: + const std::string filename; }; #else struct connection_holder { connection_holder(std::string filename, bool /*openedForeverHint*/, std::function onAfterOpen) : - filename(std::move(filename)), _onAfterOpen{std::move(onAfterOpen)} {} + _onAfterOpen{std::move(onAfterOpen)}, filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; + connection_holder(const connection_holder& other, std::function onAfterOpen) : - filename{other.filename}, _onAfterOpen{std::move(onAfterOpen)} {} + _onAfterOpen{std::move(onAfterOpen)}, filename{other.filename} {} void retain() { // first one opens the connection. // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. - if (_retain_count.fetch_add(1, std::memory_order_relaxed) == 0) { + if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open(this->filename.c_str(), &this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); @@ -14028,7 +14060,7 @@ namespace sqlite_orm { void release() { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. - if (_retain_count.fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (_retainCount.fetch_sub(1, std::memory_order_acq_rel) == 1) { int rc = sqlite3_close(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); @@ -14047,17 +14079,26 @@ namespace sqlite_orm { * @attention While retrieving the reference count value is atomic it makes only sense at single-threaded points in code. */ int retain_count() const { - return _retain_count.load(std::memory_order_relaxed); + return _retainCount.load(std::memory_order_relaxed); } - const std::string filename; - protected: - sqlite3* db = nullptr; +#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED + alignas(polyfill::hardware_destructive_interference_size) +#endif + sqlite3* db = nullptr; + + private: + std::atomic_int _retainCount{}; private: - const std::function _onAfterOpen; - std::atomic_int _retain_count{}; +#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED + alignas(polyfill::hardware_destructive_interference_size) +#endif + const std::function _onAfterOpen; + + public: + const std::string filename; }; #endif From 14a2aa0a8fd7dc73812118b19fda50095a4c2c07 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Mon, 10 Feb 2025 20:52:16 +0200 Subject: [PATCH 07/18] Ability to specify connection control options when making 'storage' A user-provided 'on open' handler and connection control options can now be specified in a declarative way when making the 'storage' object. --- dev/connection_holder.h | 12 +- dev/functional/cxx_core_features.h | 4 + dev/functional/mpl.h | 14 +- dev/storage.h | 71 +++++++++- dev/storage_base.h | 65 +++++++--- dev/tuple_helper/tuple_transformer.h | 20 ++- include/sqlite_orm/sqlite_orm.h | 186 +++++++++++++++++++++++---- tests/filename.cpp | 6 +- tests/storage_tests.cpp | 20 +++ 9 files changed, 343 insertions(+), 55 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 68d8621d0..cbb245d0f 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -66,8 +66,11 @@ namespace sqlite_orm { // first one opens and sets up the connection. - if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) - [[unlikely]] /*possible, but unexpected*/ { + if (int rc = sqlite3_open_v2(this->filename.c_str(), + &this->db, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + nullptr); + rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } @@ -143,7 +146,10 @@ namespace sqlite_orm { // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { - int rc = sqlite3_open(this->filename.c_str(), &this->db); + int rc = sqlite3_open_v2(this->filename.c_str(), + &this->db, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + nullptr); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } diff --git a/dev/functional/cxx_core_features.h b/dev/functional/cxx_core_features.h index 909698217..779737eb8 100644 --- a/dev/functional/cxx_core_features.h +++ b/dev/functional/cxx_core_features.h @@ -53,6 +53,10 @@ #define SQLITE_ORM_ALIGNED_NEW_SUPPORTED #endif +#if __cpp_deduction_guides >= 201703L +#define SQLITE_ORM_CTAD_SUPPORTED +#endif + #if __cpp_generic_lambdas >= 201707L #define SQLITE_ORM_EXPLICIT_GENERIC_LAMBDA_SUPPORTED #endif diff --git a/dev/functional/mpl.h b/dev/functional/mpl.h index f2d196dee..108f3ccd6 100644 --- a/dev/functional/mpl.h +++ b/dev/functional/mpl.h @@ -453,7 +453,7 @@ namespace sqlite_orm { * Commonly used named abbreviation for `check_if`. */ template - using check_if_is_type = mpl::bind_front_fn; + using check_if_is_type = check_if; /* * Quoted trait metafunction that checks if a type's template matches the specified template @@ -463,6 +463,18 @@ namespace sqlite_orm { using check_if_is_template = mpl::pass_extracted_fn_to>>; + /* + * Quoted trait metafunction that checks if a type names a nested type determined by `Op`. + */ + template class Op> + using check_if_names = mpl::bind_front_higherorder_fn; + + /* + * Quoted trait metafunction that checks if a type does not name a nested type determined by `Op`. + */ + template class Op> + using check_if_lacks = mpl::not_>; + /* * Quoted metafunction that finds the index of the given type in a tuple. */ diff --git a/dev/storage.h b/dev/storage.h index b9bedbf82..57848e139 100644 --- a/dev/storage.h +++ b/dev/storage.h @@ -81,21 +81,56 @@ namespace sqlite_orm { polyfill::void_t().prepare(std::declval()))>>> = true; +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template + on_open on_open_spec_or_default(PropsTpl& properties) { + if constexpr (tuple_has_type::value) { + return std::move(std::get(properties)); + } else { + return {}; + } + } + + template + connection_control connection_control_or_default(PropsTpl& properties) { + if constexpr (tuple_has_type::value) { + return std::move(std::get(properties)); + } else { + return {}; + } + } +#else + template + on_open on_open_spec_or_default(PropsTpl&) { + return {}; + } + + template + connection_control connection_control_or_default(PropsTpl&) { + return {}; + } +#endif + /** * Storage class itself. Create an instanse to use it as an interfacto to sqlite db by calling `make_storage` * function. */ template struct storage_t : storage_base { - using self = storage_t; + using self = storage_t; using db_objects_type = db_objects_tuple; /** * @param filename database filename. * @param dbObjects db_objects_tuple */ - storage_t(std::string filename, db_objects_type dbObjects) : - storage_base{std::move(filename), foreign_keys_count(dbObjects)}, db_objects{std::move(dbObjects)} {} + template + storage_t(std::string filename, db_objects_type dbObjects, PropsTpl properties) : + storage_base{std::move(filename), + on_open_spec_or_default(properties), + connection_control_or_default(properties), + foreign_keys_count(dbObjects)}, + db_objects{std::move(dbObjects)} {} storage_t(const storage_t&) = default; @@ -1702,17 +1737,45 @@ namespace sqlite_orm { } #endif // SQLITE_ORM_OPTIONAL_SUPPORTED }; // struct storage_t + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template + using storage_prop_tag_t = typename T::storage_prop_tag; + + template + using prop_index_sequence = filter_tuple_sequence_t::template fn>; + + template + using dbo_index_sequence = filter_tuple_sequence_t::template fn>; + + template + storage_t make_storage(std::string filename, std::tuple dbObjects, PropsTpl properties) { + return {std::move(filename), std::move(dbObjects), std::move(properties)}; + } +#endif } } SQLITE_ORM_EXPORT namespace sqlite_orm { +#ifdef SQLITE_ORM_CTAD_SUPPORTED /* * Factory function for a storage, from a database file and a bunch of database object definitions. */ + template + auto make_storage(std::string filename, Spec... arguments) { + using namespace ::sqlite_orm::internal; + + std::tuple args{std::forward(arguments)...}; + return make_storage(std::move(filename), + create_from_tuple(std::move(args), dbo_index_sequence{}), + create_from_tuple(std::move(args), prop_index_sequence{})); + } +#else template internal::storage_t make_storage(std::string filename, DBO... dbObjects) { - return {std::move(filename), internal::db_objects_tuple{std::forward(dbObjects)...}}; + return {std::move(filename), {std::forward(dbObjects)...}, std::tuple<>{}}; } +#endif /** * sqlite3_threadsafe() interface. diff --git a/dev/storage_base.h b/dev/storage_base.h index da4a6be82..c36df1ac9 100644 --- a/dev/storage_base.h +++ b/dev/storage_base.h @@ -35,8 +35,26 @@ #include "serializing_util.h" #include "table_info.h" -namespace sqlite_orm { +SQLITE_ORM_EXPORT namespace sqlite_orm { + struct on_open { + using storage_prop_tag = int; + +#ifndef SQLITE_ORM_AGGREGATE_PAREN_INIT_SUPPORTED + on_open() = default; + on_open(std::function onOpen) : onOpen{std::move(onOpen)} {} +#endif + std::function onOpen; + }; + + struct connection_control { + using storage_prop_tag = int; + + bool openForever = false; + }; +} + +namespace sqlite_orm { namespace internal { struct storage_base { @@ -289,17 +307,19 @@ namespace sqlite_orm { * needed and closes when it is not needed. This function breaks this rule. In memory storage always * keeps connection opened so calling this for in-memory storage changes nothing. * Note about multithreading: in multithreading context avoiding using this function for not in-memory - * storage may lead to data races. If you have data races in such a configuration try to call `open_forever` + * storage may lead to data races. If you have data races in such a configuration try to call `open_forever()` * before accessing your storage - it may fix data races. */ void open_forever() { - this->isOpenedForever = true; - this->connection->retain(); + if (!this->isOpenedForever) { + this->isOpenedForever = true; + this->connection->retain(); + } } /** * Create an application-defined scalar SQL function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_scalar_function()` merely creates a closure to generate an instance of the scalar function object, * together with a copy of the passed initialization arguments. @@ -342,7 +362,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Create an application-defined scalar function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_scalar_function()` merely creates a closure to generate an instance of the scalar function object, * together with a copy of the passed initialization arguments. @@ -357,7 +377,7 @@ namespace sqlite_orm { /** * Create an application-defined scalar function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * If `quotedF` contains a freestanding function, stateless lambda or stateless function object, * `quoted_scalar_function::callable()` uses the original function object, assuming it is free of side effects; @@ -398,7 +418,7 @@ namespace sqlite_orm { /** * Create an application-defined aggregate SQL function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_aggregate_function()` merely creates a closure to generate an instance of the aggregate function object, * together with a copy of the passed initialization arguments. @@ -447,7 +467,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Create an application-defined aggregate function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_aggregate_function()` merely creates a closure to generate an instance of the aggregate function object, * together with a copy of the passed initialization arguments. @@ -462,7 +482,7 @@ namespace sqlite_orm { /** * Delete a scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -474,7 +494,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Delete a scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -483,7 +503,7 @@ namespace sqlite_orm { /** * Delete a quoted scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -493,7 +513,7 @@ namespace sqlite_orm { /** * Delete aggregate function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_aggregate_function() { @@ -661,23 +681,31 @@ namespace sqlite_orm { } protected: - storage_base(std::string filename, int foreignKeysCount) : + storage_base(std::string filename, + sqlite_orm::on_open onOpenSpec, + connection_control connectionCtrl, + int foreignKeysCount) : + on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.openForever}, pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), connection(std::make_unique( std::move(filename), - inMemory, + inMemory || isOpenedForever, std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(foreignKeysCount) { if (this->inMemory) { this->connection->retain(); } + if (this->isOpenedForever) { + this->connection->retain(); + } } storage_base(const storage_base& other) : on_open(other.on_open), pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(other.inMemory), + isOpenedForever{other.isOpenedForever}, connection(std::make_unique( *other.connection, std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), @@ -685,6 +713,9 @@ namespace sqlite_orm { if (this->inMemory) { this->connection->retain(); } + if (this->isOpenedForever) { + this->connection->retain(); + } } ~storage_base() { @@ -719,15 +750,15 @@ namespace sqlite_orm { perform_exec(db, "PRAGMA foreign_keys", extract_single_value, &result); return result; } - #endif - void on_open_internal(sqlite3* db) { + void on_open_internal(sqlite3* db) { #if SQLITE_VERSION_NUMBER >= 3006019 if (this->cachedForeignKeysCount) { this->foreign_keys(db, true); } #endif + if (this->pragma.synchronous_ != -1) { this->pragma.set_pragma("synchronous", this->pragma.synchronous_, db); } diff --git a/dev/tuple_helper/tuple_transformer.h b/dev/tuple_helper/tuple_transformer.h index 55d940d5a..57fef52d5 100644 --- a/dev/tuple_helper/tuple_transformer.h +++ b/dev/tuple_helper/tuple_transformer.h @@ -103,7 +103,7 @@ namespace sqlite_orm { } /* - * Like `std::make_from_tuple`, but using a projection on the tuple elements. + * Like `std::make_from_tuple()`, but using a projection on the tuple elements. */ template constexpr R create_from_tuple(Tpl&& tpl, Projection project = {}) { @@ -112,5 +112,23 @@ namespace sqlite_orm { std::make_index_sequence>::value>{}, std::forward(project)); } + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template class R, class Tpl, size_t... Idx, class Projection = polyfill::identity> + constexpr auto create_from_tuple(Tpl&& tpl, std::index_sequence, Projection project = {}) { + return R{polyfill::invoke(project, std::get(std::forward(tpl)))...}; + } + + /* + * Similar to `create_from_tuple()`, but the result type is specified as a template class. + */ + template class R, class Tpl, class Projection = polyfill::identity> + constexpr auto create_from_tuple(Tpl&& tpl, Projection project = {}) { + return create_from_tuple( + std::forward(tpl), + std::make_index_sequence>::value>{}, + std::forward(project)); + } +#endif } } diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index a88693222..ee3ca0fcb 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -102,6 +102,10 @@ using std::nullptr_t; #define SQLITE_ORM_ALIGNED_NEW_SUPPORTED #endif +#if __cpp_deduction_guides >= 201703L +#define SQLITE_ORM_CTAD_SUPPORTED +#endif + #if __cpp_generic_lambdas >= 201707L #define SQLITE_ORM_EXPLICIT_GENERIC_LAMBDA_SUPPORTED #endif @@ -1242,7 +1246,7 @@ namespace sqlite_orm { * Commonly used named abbreviation for `check_if`. */ template - using check_if_is_type = mpl::bind_front_fn; + using check_if_is_type = check_if; /* * Quoted trait metafunction that checks if a type's template matches the specified template @@ -1252,6 +1256,18 @@ namespace sqlite_orm { using check_if_is_template = mpl::pass_extracted_fn_to>>; + /* + * Quoted trait metafunction that checks if a type names a nested type determined by `Op`. + */ + template class Op> + using check_if_names = mpl::bind_front_higherorder_fn; + + /* + * Quoted trait metafunction that checks if a type does not name a nested type determined by `Op`. + */ + template class Op> + using check_if_lacks = mpl::not_>; + /* * Quoted metafunction that finds the index of the given type in a tuple. */ @@ -1643,7 +1659,7 @@ namespace sqlite_orm { } /* - * Like `std::make_from_tuple`, but using a projection on the tuple elements. + * Like `std::make_from_tuple()`, but using a projection on the tuple elements. */ template constexpr R create_from_tuple(Tpl&& tpl, Projection project = {}) { @@ -1652,6 +1668,24 @@ namespace sqlite_orm { std::make_index_sequence>::value>{}, std::forward(project)); } + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template class R, class Tpl, size_t... Idx, class Projection = polyfill::identity> + constexpr auto create_from_tuple(Tpl&& tpl, std::index_sequence, Projection project = {}) { + return R{polyfill::invoke(project, std::get(std::forward(tpl)))...}; + } + + /* + * Similar to `create_from_tuple()`, but the result type is specified as a template class. + */ + template class R, class Tpl, class Projection = polyfill::identity> + constexpr auto create_from_tuple(Tpl&& tpl, Projection project = {}) { + return create_from_tuple( + std::forward(tpl), + std::make_index_sequence>::value>{}, + std::forward(project)); + } +#endif } } @@ -13969,8 +14003,11 @@ namespace sqlite_orm { // first one opens and sets up the connection. - if (int rc = sqlite3_open(this->filename.c_str(), &this->db); rc != SQLITE_OK) - [[unlikely]] /*possible, but unexpected*/ { + if (int rc = sqlite3_open_v2(this->filename.c_str(), + &this->db, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + nullptr); + rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } @@ -14046,7 +14083,10 @@ namespace sqlite_orm { // we presume that the connection is opened once in a single-threaded context [also open forever]. // therefore we can just use an atomic increment but don't need sequencing due to `prevCount > 0`. if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { - int rc = sqlite3_open(this->filename.c_str(), &this->db); + int rc = sqlite3_open_v2(this->filename.c_str(), + &this->db, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + nullptr); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } @@ -17877,8 +17917,26 @@ namespace sqlite_orm { // #include "table_info.h" -namespace sqlite_orm { +SQLITE_ORM_EXPORT namespace sqlite_orm { + struct on_open { + using storage_prop_tag = int; + +#ifndef SQLITE_ORM_AGGREGATE_PAREN_INIT_SUPPORTED + on_open() = default; + on_open(std::function onOpen) : onOpen{std::move(onOpen)} {} +#endif + + std::function onOpen; + }; + + struct connection_control { + using storage_prop_tag = int; + bool openForever = false; + }; +} + +namespace sqlite_orm { namespace internal { struct storage_base { @@ -18131,17 +18189,19 @@ namespace sqlite_orm { * needed and closes when it is not needed. This function breaks this rule. In memory storage always * keeps connection opened so calling this for in-memory storage changes nothing. * Note about multithreading: in multithreading context avoiding using this function for not in-memory - * storage may lead to data races. If you have data races in such a configuration try to call `open_forever` + * storage may lead to data races. If you have data races in such a configuration try to call `open_forever()` * before accessing your storage - it may fix data races. */ void open_forever() { - this->isOpenedForever = true; - this->connection->retain(); + if (!this->isOpenedForever) { + this->isOpenedForever = true; + this->connection->retain(); + } } /** * Create an application-defined scalar SQL function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_scalar_function()` merely creates a closure to generate an instance of the scalar function object, * together with a copy of the passed initialization arguments. @@ -18184,7 +18244,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Create an application-defined scalar function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_scalar_function()` merely creates a closure to generate an instance of the scalar function object, * together with a copy of the passed initialization arguments. @@ -18199,7 +18259,7 @@ namespace sqlite_orm { /** * Create an application-defined scalar function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * If `quotedF` contains a freestanding function, stateless lambda or stateless function object, * `quoted_scalar_function::callable()` uses the original function object, assuming it is free of side effects; @@ -18240,7 +18300,7 @@ namespace sqlite_orm { /** * Create an application-defined aggregate SQL function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_aggregate_function()` merely creates a closure to generate an instance of the aggregate function object, * together with a copy of the passed initialization arguments. @@ -18289,7 +18349,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Create an application-defined aggregate function. - * Can be called at any time no matter whether the database connection is opened or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is opened or not. * * Note: `create_aggregate_function()` merely creates a closure to generate an instance of the aggregate function object, * together with a copy of the passed initialization arguments. @@ -18304,7 +18364,7 @@ namespace sqlite_orm { /** * Delete a scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -18316,7 +18376,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_WITH_CPP20_ALIASES /** * Delete a scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -18325,7 +18385,7 @@ namespace sqlite_orm { /** * Delete a quoted scalar function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_scalar_function() { @@ -18335,7 +18395,7 @@ namespace sqlite_orm { /** * Delete aggregate function you created before. - * Can be called at any time no matter whether the database connection is open or not. + * Can be called at any time (in a single-threaded context) no matter whether the database connection is open or not. */ template void delete_aggregate_function() { @@ -18503,23 +18563,31 @@ namespace sqlite_orm { } protected: - storage_base(std::string filename, int foreignKeysCount) : + storage_base(std::string filename, + sqlite_orm::on_open onOpenSpec, + connection_control connectionCtrl, + int foreignKeysCount) : + on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.openForever}, pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), connection(std::make_unique( std::move(filename), - inMemory, + inMemory || isOpenedForever, std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), cachedForeignKeysCount(foreignKeysCount) { if (this->inMemory) { this->connection->retain(); } + if (this->isOpenedForever) { + this->connection->retain(); + } } storage_base(const storage_base& other) : on_open(other.on_open), pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(other.inMemory), + isOpenedForever{other.isOpenedForever}, connection(std::make_unique( *other.connection, std::bind(&storage_base::on_open_internal, this, std::placeholders::_1))), @@ -18527,6 +18595,9 @@ namespace sqlite_orm { if (this->inMemory) { this->connection->retain(); } + if (this->isOpenedForever) { + this->connection->retain(); + } } ~storage_base() { @@ -18561,15 +18632,15 @@ namespace sqlite_orm { perform_exec(db, "PRAGMA foreign_keys", extract_single_value, &result); return result; } - #endif - void on_open_internal(sqlite3* db) { + void on_open_internal(sqlite3* db) { #if SQLITE_VERSION_NUMBER >= 3006019 if (this->cachedForeignKeysCount) { this->foreign_keys(db, true); } #endif + if (this->pragma.synchronous_ != -1) { this->pragma.set_pragma("synchronous", this->pragma.synchronous_, db); } @@ -22602,21 +22673,56 @@ namespace sqlite_orm { polyfill::void_t().prepare(std::declval()))>>> = true; +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template + on_open on_open_spec_or_default(PropsTpl& properties) { + if constexpr (tuple_has_type::value) { + return std::move(std::get(properties)); + } else { + return {}; + } + } + + template + connection_control connection_control_or_default(PropsTpl& properties) { + if constexpr (tuple_has_type::value) { + return std::move(std::get(properties)); + } else { + return {}; + } + } +#else + template + on_open on_open_spec_or_default(PropsTpl&) { + return {}; + } + + template + connection_control connection_control_or_default(PropsTpl&) { + return {}; + } +#endif + /** * Storage class itself. Create an instanse to use it as an interfacto to sqlite db by calling `make_storage` * function. */ template struct storage_t : storage_base { - using self = storage_t; + using self = storage_t; using db_objects_type = db_objects_tuple; /** * @param filename database filename. * @param dbObjects db_objects_tuple */ - storage_t(std::string filename, db_objects_type dbObjects) : - storage_base{std::move(filename), foreign_keys_count(dbObjects)}, db_objects{std::move(dbObjects)} {} + template + storage_t(std::string filename, db_objects_type dbObjects, PropsTpl properties) : + storage_base{std::move(filename), + on_open_spec_or_default(properties), + connection_control_or_default(properties), + foreign_keys_count(dbObjects)}, + db_objects{std::move(dbObjects)} {} storage_t(const storage_t&) = default; @@ -24223,17 +24329,45 @@ namespace sqlite_orm { } #endif // SQLITE_ORM_OPTIONAL_SUPPORTED }; // struct storage_t + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + template + using storage_prop_tag_t = typename T::storage_prop_tag; + + template + using prop_index_sequence = filter_tuple_sequence_t::template fn>; + + template + using dbo_index_sequence = filter_tuple_sequence_t::template fn>; + + template + storage_t make_storage(std::string filename, std::tuple dbObjects, PropsTpl properties) { + return {std::move(filename), std::move(dbObjects), std::move(properties)}; + } +#endif } } SQLITE_ORM_EXPORT namespace sqlite_orm { +#ifdef SQLITE_ORM_CTAD_SUPPORTED /* * Factory function for a storage, from a database file and a bunch of database object definitions. */ + template + auto make_storage(std::string filename, Spec... arguments) { + using namespace ::sqlite_orm::internal; + + std::tuple args{std::forward(arguments)...}; + return make_storage(std::move(filename), + create_from_tuple(std::move(args), dbo_index_sequence{}), + create_from_tuple(std::move(args), prop_index_sequence{})); + } +#else template internal::storage_t make_storage(std::string filename, DBO... dbObjects) { - return {std::move(filename), internal::db_objects_tuple{std::forward(dbObjects)...}}; + return {std::move(filename), {std::forward(dbObjects)...}, std::tuple<>{}}; } +#endif /** * sqlite3_threadsafe() interface. diff --git a/tests/filename.cpp b/tests/filename.cpp index e779ab836..de4f5559f 100644 --- a/tests/filename.cpp +++ b/tests/filename.cpp @@ -4,15 +4,15 @@ using namespace sqlite_orm; TEST_CASE("filename") { - { + SECTION("empty") { auto storage = make_storage(""); REQUIRE(storage.filename() == ""); } - { + SECTION("memory") { auto storage = make_storage(":memory:"); REQUIRE(storage.filename() == ":memory:"); } - { + SECTION("file name") { auto storage = make_storage("myDatabase.sqlite"); REQUIRE(storage.filename() == "myDatabase.sqlite"); } diff --git a/tests/storage_tests.cpp b/tests/storage_tests.cpp index c46cd28ad..570285a0e 100644 --- a/tests/storage_tests.cpp +++ b/tests/storage_tests.cpp @@ -4,6 +4,26 @@ using namespace sqlite_orm; +#ifdef SQLITE_ORM_CTAD_SUPPORTED +TEST_CASE("connection control") { + const auto openForever = GENERATE(false, true); + SECTION("") { + SECTION("empty") { + auto storage = make_storage("", connection_control{openForever}, on_open([](sqlite3*) {})); + REQUIRE(storage.is_opened()); + } + SECTION("memory") { + auto storage = make_storage(":memory:", connection_control{openForever}, on_open([](sqlite3*) {})); + REQUIRE(storage.is_opened()); + } + SECTION("file name") { + auto storage = make_storage("myDatabase.sqlite", connection_control{openForever}, on_open([](sqlite3*) {})); + REQUIRE(storage.is_opened() == openForever); + } + } +} +#endif + TEST_CASE("Current time/date/timestamp") { auto storage = make_storage(""); SECTION("time") { From de7cee02aaaf41fe677b2595ddeb3eefbbcbff48 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Tue, 11 Feb 2025 09:18:47 +0200 Subject: [PATCH 08/18] Corrected a pre-processor condition --- dev/connection_holder.h | 4 ++-- include/sqlite_orm/sqlite_orm.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index cbb245d0f..6c64033ed 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -186,7 +186,7 @@ namespace sqlite_orm { } protected: -#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif sqlite3* db = nullptr; @@ -195,7 +195,7 @@ namespace sqlite_orm { std::atomic_int _retainCount{}; private: -#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif const std::function _onAfterOpen; diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index ee3ca0fcb..3196bb05d 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -14123,7 +14123,7 @@ namespace sqlite_orm { } protected: -#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif sqlite3* db = nullptr; @@ -14132,7 +14132,7 @@ namespace sqlite_orm { std::atomic_int _retainCount{}; private: -#if SQLITE_ORM_ALIGNED_NEW_SUPPORTED +#ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif const std::function _onAfterOpen; From 9f5654f2d7aaff4769fc3488924f299627922e8c Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Thu, 13 Feb 2025 16:12:07 +0200 Subject: [PATCH 09/18] Added another static unit test for `create_from_tuple()` --- tests/static_tests/functional/tuple_transform.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/static_tests/functional/tuple_transform.cpp b/tests/static_tests/functional/tuple_transform.cpp index d67539098..fe336a289 100644 --- a/tests/static_tests/functional/tuple_transform.cpp +++ b/tests/static_tests/functional/tuple_transform.cpp @@ -49,6 +49,10 @@ TEST_CASE("tuple_helper static") { #if __cpp_lib_constexpr_algorithms >= 201806L STATIC_REQUIRE(create_from_tuple>(std::make_tuple(1, 2), polyfill::identity{}) == std::array{1, 2}); + STATIC_REQUIRE(create_from_tuple(std::make_tuple(1, 2), polyfill::identity{}) == + std::tuple{1, 2}); + STATIC_REQUIRE(create_from_tuple(std::make_tuple(1, 2), polyfill::identity{}) == + std::array{1, 2}); #endif #if defined(SQLITE_ORM_FOLD_EXPRESSIONS_SUPPORTED) && (__cpp_lib_constexpr_functional >= 201907L) STATIC_REQUIRE(recombine_tuple(tuple_maker{}, std::make_tuple(1, 2), polyfill::identity{}, 3) == From ef12ae43d96027614f55ae70851a07e628dfadb0 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sun, 16 Feb 2025 14:17:24 +0200 Subject: [PATCH 10/18] Corrected UT for tuple transformation --- tests/static_tests/functional/tuple_transform.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/static_tests/functional/tuple_transform.cpp b/tests/static_tests/functional/tuple_transform.cpp index fe336a289..26bcab109 100644 --- a/tests/static_tests/functional/tuple_transform.cpp +++ b/tests/static_tests/functional/tuple_transform.cpp @@ -51,8 +51,6 @@ TEST_CASE("tuple_helper static") { std::array{1, 2}); STATIC_REQUIRE(create_from_tuple(std::make_tuple(1, 2), polyfill::identity{}) == std::tuple{1, 2}); - STATIC_REQUIRE(create_from_tuple(std::make_tuple(1, 2), polyfill::identity{}) == - std::array{1, 2}); #endif #if defined(SQLITE_ORM_FOLD_EXPRESSIONS_SUPPORTED) && (__cpp_lib_constexpr_functional >= 201907L) STATIC_REQUIRE(recombine_tuple(tuple_maker{}, std::make_tuple(1, 2), polyfill::identity{}, 3) == From 3ed75071b3533deffc8695278ce304562f885360 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sun, 16 Feb 2025 14:29:12 +0200 Subject: [PATCH 11/18] Renamed public-facing connection control member variable --- dev/storage.h | 71 ++++++----------- dev/storage_base.h | 33 ++------ dev/storage_options.h | 50 ++++++++++++ include/sqlite_orm/sqlite_orm.h | 136 +++++++++++++++++--------------- tests/storage_tests.cpp | 29 ++++++- 5 files changed, 183 insertions(+), 136 deletions(-) create mode 100644 dev/storage_options.h diff --git a/dev/storage.h b/dev/storage.h index 57848e139..89e2f0540 100644 --- a/dev/storage.h +++ b/dev/storage.h @@ -81,35 +81,18 @@ namespace sqlite_orm { polyfill::void_t().prepare(std::declval()))>>> = true; + template + decltype(auto) storage_opt_or_default(OptionsTpl& options) { #ifdef SQLITE_ORM_CTAD_SUPPORTED - template - on_open on_open_spec_or_default(PropsTpl& properties) { - if constexpr (tuple_has_type::value) { - return std::move(std::get(properties)); + if constexpr (tuple_has_type::value) { + return std::move(std::get(options)); } else { - return {}; + return Opt{}; } - } - - template - connection_control connection_control_or_default(PropsTpl& properties) { - if constexpr (tuple_has_type::value) { - return std::move(std::get(properties)); - } else { - return {}; - } - } #else - template - on_open on_open_spec_or_default(PropsTpl&) { - return {}; - } - - template - connection_control connection_control_or_default(PropsTpl&) { - return {}; - } + return Opt{}; #endif + } /** * Storage class itself. Create an instanse to use it as an interfacto to sqlite db by calling `make_storage` @@ -117,18 +100,18 @@ namespace sqlite_orm { */ template struct storage_t : storage_base { - using self = storage_t; + using self_type = storage_t; using db_objects_type = db_objects_tuple; /** * @param filename database filename. * @param dbObjects db_objects_tuple */ - template - storage_t(std::string filename, db_objects_type dbObjects, PropsTpl properties) : + template + storage_t(std::string filename, db_objects_type dbObjects, OptionsTpl options) : storage_base{std::move(filename), - on_open_spec_or_default(properties), - connection_control_or_default(properties), + storage_opt_or_default(options), + storage_opt_or_default(options), foreign_keys_count(dbObjects)}, db_objects{std::move(dbObjects)} {} @@ -149,7 +132,7 @@ namespace sqlite_orm { * * Hence, friend was replaced by `obtain_db_objects()` and `pick_const_impl()`. */ - friend const db_objects_type& obtain_db_objects(const self& storage) noexcept { + friend const db_objects_type& obtain_db_objects(const self_type& storage) noexcept { return storage.db_objects; } @@ -281,7 +264,7 @@ namespace sqlite_orm { public: template, class... Args> - mapped_view iterate(Args&&... args) { + mapped_view iterate(Args&&... args) { this->assert_mapped_type(); auto con = this->get_connection(); @@ -816,7 +799,7 @@ namespace sqlite_orm { std::enable_if_t::value && !is_mapped::value, bool> = true> std::string dump(E&& expression, bool parametrized = false) const { - static_assert(is_preparable_v, "Expression must be a high-level statement"); + static_assert(is_preparable_v, "Expression must be a high-level statement"); decltype(auto) e2 = static_if::value>( [](auto expression) -> auto { @@ -1739,18 +1722,15 @@ namespace sqlite_orm { }; // struct storage_t #ifdef SQLITE_ORM_CTAD_SUPPORTED - template - using storage_prop_tag_t = typename T::storage_prop_tag; - template - using prop_index_sequence = filter_tuple_sequence_t::template fn>; + using dbo_index_sequence = filter_tuple_sequence_t::template fn>; template - using dbo_index_sequence = filter_tuple_sequence_t::template fn>; + using opt_index_sequence = filter_tuple_sequence_t::template fn>; - template - storage_t make_storage(std::string filename, std::tuple dbObjects, PropsTpl properties) { - return {std::move(filename), std::move(dbObjects), std::move(properties)}; + template + storage_t make_storage(std::string filename, std::tuple dbObjects, OptionsTpl options) { + return {std::move(filename), std::move(dbObjects), std::move(options)}; } #endif } @@ -1762,13 +1742,14 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { * Factory function for a storage, from a database file and a bunch of database object definitions. */ template - auto make_storage(std::string filename, Spec... arguments) { + auto make_storage(std::string filename, Spec... specifications) { using namespace ::sqlite_orm::internal; - std::tuple args{std::forward(arguments)...}; - return make_storage(std::move(filename), - create_from_tuple(std::move(args), dbo_index_sequence{}), - create_from_tuple(std::move(args), prop_index_sequence{})); + std::tuple specTuple{std::forward(specifications)...}; + return internal::make_storage( + std::move(filename), + create_from_tuple(std::move(specTuple), dbo_index_sequence{}), + create_from_tuple(std::move(specTuple), opt_index_sequence{})); } #else template diff --git a/dev/storage_base.h b/dev/storage_base.h index c36df1ac9..bf0239164 100644 --- a/dev/storage_base.h +++ b/dev/storage_base.h @@ -14,7 +14,7 @@ #include // std::list #include // std::make_unique, std::unique_ptr #include // std::map -#include // std::is_same +#include // std::is_same, std::is_aggregate #include // std::find_if, std::ranges::find #endif @@ -34,25 +34,7 @@ #include "udf_proxy.h" #include "serializing_util.h" #include "table_info.h" - -SQLITE_ORM_EXPORT namespace sqlite_orm { - struct on_open { - using storage_prop_tag = int; - -#ifndef SQLITE_ORM_AGGREGATE_PAREN_INIT_SUPPORTED - on_open() = default; - on_open(std::function onOpen) : onOpen{std::move(onOpen)} {} -#endif - - std::function onOpen; - }; - - struct connection_control { - using storage_prop_tag = int; - - bool openForever = false; - }; -} +#include "storage_options.h" namespace sqlite_orm { namespace internal { @@ -682,10 +664,10 @@ namespace sqlite_orm { protected: storage_base(std::string filename, - sqlite_orm::on_open onOpenSpec, connection_control connectionCtrl, + on_open_spec onOpenSpec, int foreignKeysCount) : - on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.openForever}, + on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.open_forever}, pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), @@ -1020,8 +1002,8 @@ namespace sqlite_orm { }); #endif if (dbColumnInfoIt != dbTableInfo.end()) { - auto& dbColumnInfo = *dbColumnInfoIt; - auto columnsAreEqual = + table_xinfo& dbColumnInfo = *dbColumnInfoIt; + bool columnsAreEqual = dbColumnInfo.name == storageColumnInfo.name && dbColumnInfo.notnull == storageColumnInfo.notnull && (!dbColumnInfo.dflt_value.empty()) == (!storageColumnInfo.dflt_value.empty()) && @@ -1032,8 +1014,7 @@ namespace sqlite_orm { break; } dbTableInfo.erase(dbColumnInfoIt); - storageTableInfo.erase(storageTableInfo.begin() + - static_cast(storageColumnInfoIndex)); + storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex); --storageColumnInfoIndex; } else { columnsToAdd.push_back(&storageColumnInfo); diff --git a/dev/storage_options.h b/dev/storage_options.h new file mode 100644 index 000000000..54f310087 --- /dev/null +++ b/dev/storage_options.h @@ -0,0 +1,50 @@ +#pragma once + +#include +#ifdef SQLITE_ORM_IMPORT_STD_MODULE +#include +#else +#include // std::is_aggregate +#include // std::move +#include // std::function +#endif + +namespace sqlite_orm { + namespace internal { + template + using storage_opt_tag_t = typename T::storage_opt_tag; + + struct on_open_spec { + using storage_opt_tag = int; + + std::function onOpen; + }; + } +} + +SQLITE_ORM_EXPORT namespace sqlite_orm { + /** + * Database connection control options to be passed to `make_storage()`. + */ + struct connection_control { + /// Whether to open the database once and for all. + /// Required if using a 'storage' instance from multiple threads. + bool open_forever = false; + + using storage_opt_tag = int; + }; +#if __cpp_lib_is_aggregate >= 201703L + // design choice: must be an aggregate that can be constructed using designated initializers + static_assert(std::is_aggregate_v); +#endif + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + /** + * Callback function to be passed to `make_storage()`. + * The provided function is called immdediately after the database connection has been established and set up. + */ + inline internal::on_open_spec on_open(std::function onOpen) { + return {std::move(onOpen)}; + } +#endif +} diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 3196bb05d..04a32b3ee 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -16400,7 +16400,7 @@ inline constexpr bool std::ranges::enable_borrowed_range // std::list #include // std::make_unique, std::unique_ptr #include // std::map -#include // std::is_same +#include // std::is_same, std::is_aggregate #include // std::find_if, std::ranges::find #endif @@ -17917,23 +17917,55 @@ namespace sqlite_orm { // #include "table_info.h" -SQLITE_ORM_EXPORT namespace sqlite_orm { - struct on_open { - using storage_prop_tag = int; +// #include "storage_options.h" -#ifndef SQLITE_ORM_AGGREGATE_PAREN_INIT_SUPPORTED - on_open() = default; - on_open(std::function onOpen) : onOpen{std::move(onOpen)} {} +#include +#ifdef SQLITE_ORM_IMPORT_STD_MODULE +#include +#else +#include // std::is_aggregate +#include // std::move +#include // std::function #endif - std::function onOpen; - }; +namespace sqlite_orm { + namespace internal { + template + using storage_opt_tag_t = typename T::storage_opt_tag; + + struct on_open_spec { + using storage_opt_tag = int; + std::function onOpen; + }; + } +} + +SQLITE_ORM_EXPORT namespace sqlite_orm { + /** + * Database connection control options to be passed to `make_storage()`. + */ struct connection_control { - using storage_prop_tag = int; + /// Whether to open the database once and for all. + /// Required if using a 'storage' instance from multiple threads. + bool open_forever = false; - bool openForever = false; + using storage_opt_tag = int; }; +#if __cpp_lib_is_aggregate >= 201703L + // design choice: must be an aggregate that can be constructed using designated initializers + static_assert(std::is_aggregate_v); +#endif + +#ifdef SQLITE_ORM_CTAD_SUPPORTED + /** + * Callback function to be passed to `make_storage()`. + * The provided function is called immdediately after the database connection has been established and set up. + */ + inline internal::on_open_spec on_open(std::function onOpen) { + return {std::move(onOpen)}; + } +#endif } namespace sqlite_orm { @@ -18564,10 +18596,10 @@ namespace sqlite_orm { protected: storage_base(std::string filename, - sqlite_orm::on_open onOpenSpec, connection_control connectionCtrl, + on_open_spec onOpenSpec, int foreignKeysCount) : - on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.openForever}, + on_open{std::move(onOpenSpec.onOpen)}, isOpenedForever{connectionCtrl.open_forever}, pragma(std::bind(&storage_base::get_connection, this)), limit(std::bind(&storage_base::get_connection, this)), inMemory(filename.empty() || filename == ":memory:"), @@ -18902,8 +18934,8 @@ namespace sqlite_orm { }); #endif if (dbColumnInfoIt != dbTableInfo.end()) { - auto& dbColumnInfo = *dbColumnInfoIt; - auto columnsAreEqual = + table_xinfo& dbColumnInfo = *dbColumnInfoIt; + bool columnsAreEqual = dbColumnInfo.name == storageColumnInfo.name && dbColumnInfo.notnull == storageColumnInfo.notnull && (!dbColumnInfo.dflt_value.empty()) == (!storageColumnInfo.dflt_value.empty()) && @@ -18914,8 +18946,7 @@ namespace sqlite_orm { break; } dbTableInfo.erase(dbColumnInfoIt); - storageTableInfo.erase(storageTableInfo.begin() + - static_cast(storageColumnInfoIndex)); + storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex); --storageColumnInfoIndex; } else { columnsToAdd.push_back(&storageColumnInfo); @@ -22673,35 +22704,18 @@ namespace sqlite_orm { polyfill::void_t().prepare(std::declval()))>>> = true; + template + decltype(auto) storage_opt_or_default(OptionsTpl& options) { #ifdef SQLITE_ORM_CTAD_SUPPORTED - template - on_open on_open_spec_or_default(PropsTpl& properties) { - if constexpr (tuple_has_type::value) { - return std::move(std::get(properties)); - } else { - return {}; - } - } - - template - connection_control connection_control_or_default(PropsTpl& properties) { - if constexpr (tuple_has_type::value) { - return std::move(std::get(properties)); + if constexpr (tuple_has_type::value) { + return std::move(std::get(options)); } else { - return {}; + return Opt{}; } - } #else - template - on_open on_open_spec_or_default(PropsTpl&) { - return {}; - } - - template - connection_control connection_control_or_default(PropsTpl&) { - return {}; - } + return Opt{}; #endif + } /** * Storage class itself. Create an instanse to use it as an interfacto to sqlite db by calling `make_storage` @@ -22709,18 +22723,18 @@ namespace sqlite_orm { */ template struct storage_t : storage_base { - using self = storage_t; + using self_type = storage_t; using db_objects_type = db_objects_tuple; /** * @param filename database filename. * @param dbObjects db_objects_tuple */ - template - storage_t(std::string filename, db_objects_type dbObjects, PropsTpl properties) : + template + storage_t(std::string filename, db_objects_type dbObjects, OptionsTpl options) : storage_base{std::move(filename), - on_open_spec_or_default(properties), - connection_control_or_default(properties), + storage_opt_or_default(options), + storage_opt_or_default(options), foreign_keys_count(dbObjects)}, db_objects{std::move(dbObjects)} {} @@ -22741,7 +22755,7 @@ namespace sqlite_orm { * * Hence, friend was replaced by `obtain_db_objects()` and `pick_const_impl()`. */ - friend const db_objects_type& obtain_db_objects(const self& storage) noexcept { + friend const db_objects_type& obtain_db_objects(const self_type& storage) noexcept { return storage.db_objects; } @@ -22873,7 +22887,7 @@ namespace sqlite_orm { public: template, class... Args> - mapped_view iterate(Args&&... args) { + mapped_view iterate(Args&&... args) { this->assert_mapped_type(); auto con = this->get_connection(); @@ -23408,7 +23422,7 @@ namespace sqlite_orm { std::enable_if_t::value && !is_mapped::value, bool> = true> std::string dump(E&& expression, bool parametrized = false) const { - static_assert(is_preparable_v, "Expression must be a high-level statement"); + static_assert(is_preparable_v, "Expression must be a high-level statement"); decltype(auto) e2 = static_if::value>( [](auto expression) -> auto { @@ -24331,18 +24345,15 @@ namespace sqlite_orm { }; // struct storage_t #ifdef SQLITE_ORM_CTAD_SUPPORTED - template - using storage_prop_tag_t = typename T::storage_prop_tag; - template - using prop_index_sequence = filter_tuple_sequence_t::template fn>; + using dbo_index_sequence = filter_tuple_sequence_t::template fn>; template - using dbo_index_sequence = filter_tuple_sequence_t::template fn>; + using opt_index_sequence = filter_tuple_sequence_t::template fn>; - template - storage_t make_storage(std::string filename, std::tuple dbObjects, PropsTpl properties) { - return {std::move(filename), std::move(dbObjects), std::move(properties)}; + template + storage_t make_storage(std::string filename, std::tuple dbObjects, OptionsTpl options) { + return {std::move(filename), std::move(dbObjects), std::move(options)}; } #endif } @@ -24354,13 +24365,14 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { * Factory function for a storage, from a database file and a bunch of database object definitions. */ template - auto make_storage(std::string filename, Spec... arguments) { + auto make_storage(std::string filename, Spec... specifications) { using namespace ::sqlite_orm::internal; - std::tuple args{std::forward(arguments)...}; - return make_storage(std::move(filename), - create_from_tuple(std::move(args), dbo_index_sequence{}), - create_from_tuple(std::move(args), prop_index_sequence{})); + std::tuple specTuple{std::forward(specifications)...}; + return internal::make_storage( + std::move(filename), + create_from_tuple(std::move(specTuple), dbo_index_sequence{}), + create_from_tuple(std::move(specTuple), opt_index_sequence{})); } #else template diff --git a/tests/storage_tests.cpp b/tests/storage_tests.cpp index 570285a0e..0a3598641 100644 --- a/tests/storage_tests.cpp +++ b/tests/storage_tests.cpp @@ -8,17 +8,40 @@ using namespace sqlite_orm; TEST_CASE("connection control") { const auto openForever = GENERATE(false, true); SECTION("") { + bool onOpenCalled = false; SECTION("empty") { - auto storage = make_storage("", connection_control{openForever}, on_open([](sqlite3*) {})); + auto storage = make_storage("", connection_control{openForever}, on_open([&onOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + })); REQUIRE(storage.is_opened()); + REQUIRE(onOpenCalled); } +#if __cpp_designated_initializers >= 201707L + SECTION("empty C++20") { + auto storage = + make_storage("", connection_control{.open_forever = openForever}, on_open([&onOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + })); + REQUIRE(storage.is_opened()); + REQUIRE(onOpenCalled); + } +#endif SECTION("memory") { - auto storage = make_storage(":memory:", connection_control{openForever}, on_open([](sqlite3*) {})); + auto storage = + make_storage(":memory:", connection_control{openForever}, on_open([&onOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + })); REQUIRE(storage.is_opened()); + REQUIRE(onOpenCalled); } SECTION("file name") { - auto storage = make_storage("myDatabase.sqlite", connection_control{openForever}, on_open([](sqlite3*) {})); + auto storage = make_storage("myDatabase.sqlite", + connection_control{openForever}, + on_open([&onOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + })); REQUIRE(storage.is_opened() == openForever); + REQUIRE(onOpenCalled == openForever); } } } From 78bd3ff0e4ef9f82982d68b23f8541237e22d8dd Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Wed, 19 Feb 2025 12:43:15 +0200 Subject: [PATCH 12/18] Merge branch 'upstream/dev' into upstream/experimental/threadsafe_connection --- dev/connection_holder.h | 39 ++++++++++---------- dev/storage.h | 6 +++- dev/storage_impl.h | 19 +++++----- include/sqlite_orm/sqlite_orm.h | 64 ++++++++++++++++----------------- tests/storage_tests.cpp | 36 ++++++++++++------- 5 files changed, 88 insertions(+), 76 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 6c64033ed..c89bd4780 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -44,15 +44,15 @@ namespace sqlite_orm { std::binary_semaphore& sync; }; - connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : - _openedForeverHint{openedForeverHint}, _onAfterOpen{std::move(onAfterOpen)}, - filename(std::move(filename)) {} + connection_holder(std::string filename, bool openedForeverHint, std::function didOpenDb) : + _openedForeverHint{openedForeverHint}, _didOpenDb{std::move(didOpenDb)}, filename(std::move(filename)) { + } connection_holder(const connection_holder&) = delete; - connection_holder(const connection_holder& other, std::function onAfterOpen) : - filename{other.filename}, _openedForeverHint{other._openedForeverHint}, - _onAfterOpen{std::move(onAfterOpen)} {} + connection_holder(const connection_holder& other, std::function didOpenDb) : + _openedForeverHint{other._openedForeverHint}, _didOpenDb{std::move(didOpenDb)}, + filename{other.filename} {} void retain() { const maybe_lock maybeLock{_sync, !_openedForeverHint}; @@ -74,8 +74,8 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } - if (_onAfterOpen) { - _onAfterOpen(this->db); + if (_didOpenDb) { + _didOpenDb(this->db); } } @@ -95,7 +95,7 @@ namespace sqlite_orm { // last one closes the connection. - if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { + if (int rc = sqlite3_close_v2(this->db); rc != SQLITE_OK) [[unlikely]] { throw_translated_sqlite_error(this->db); } else { this->db = nullptr; @@ -123,8 +123,7 @@ namespace sqlite_orm { std::binary_semaphore _sync{1}; private: - alignas( - polyfill::hardware_destructive_interference_size) const std::function _onAfterOpen; + alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; public: const std::string filename; @@ -133,13 +132,13 @@ namespace sqlite_orm { struct connection_holder { connection_holder(std::string filename, bool /*openedForeverHint*/, - std::function onAfterOpen) : - _onAfterOpen{std::move(onAfterOpen)}, filename(std::move(filename)) {} + std::function didOpenDb) : + _didOpenDb{std::move(didOpenDb)}, filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; - connection_holder(const connection_holder& other, std::function onAfterOpen) : - _onAfterOpen{std::move(onAfterOpen)}, filename{other.filename} {} + connection_holder(const connection_holder& other, std::function didOpenDb) : + _didOpenDb{std::move(didOpenDb)}, filename{other.filename} {} void retain() { // first one opens the connection. @@ -153,10 +152,10 @@ namespace sqlite_orm { if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } - } - if (_onAfterOpen) { - _onAfterOpen(this->db); + if (_didOpenDb) { + _didOpenDb(this->db); + } } } @@ -164,7 +163,7 @@ namespace sqlite_orm { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. if (_retainCount.fetch_sub(1, std::memory_order_acq_rel) == 1) { - int rc = sqlite3_close(this->db); + int rc = sqlite3_close_v2(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); } else { @@ -198,7 +197,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif - const std::function _onAfterOpen; + const std::function _didOpenDb; public: const std::string filename; diff --git a/dev/storage.h b/dev/storage.h index 89e2f0540..f0d97dda7 100644 --- a/dev/storage.h +++ b/dev/storage.h @@ -1739,7 +1739,11 @@ namespace sqlite_orm { SQLITE_ORM_EXPORT namespace sqlite_orm { #ifdef SQLITE_ORM_CTAD_SUPPORTED /* - * Factory function for a storage, from a database file and a bunch of database object definitions. + * Factory function for a storage instance, from a database file, a set of database object definitions + * and option storage options like connection control options and an 'on open' callback. + * + * E.g. + * auto storage = make_storage("", connection_control{.open_forever = true}, on_open([](sqlite3* db) {})); */ template auto make_storage(std::string filename, Spec... specifications) { diff --git a/dev/storage_impl.h b/dev/storage_impl.h index 81537fdcc..b90efd1e5 100644 --- a/dev/storage_impl.h +++ b/dev/storage_impl.h @@ -70,17 +70,15 @@ namespace sqlite_orm { constexpr decltype(auto) materialize_column_pointer(const DBOs&, const column_pointer>&) { using table_type = storage_pick_table_t; - using cte_mapper_type = cte_mapper_type_t; + using cte_colrefs_tuple = typename cte_mapper_type_t::final_colrefs_tuple; + using cte_fields_type = typename cte_mapper_type_t::fields_type; // lookup ColAlias in the final column references - using colalias_index = - find_tuple_type>; - static_assert(colalias_index::value < std::tuple_size_v, + using colalias_index = find_tuple_type>; + static_assert(colalias_index::value < std::tuple_size_v, "No such column mapped into the CTE."); - return &aliased_field< - ColAlias, - std::tuple_element_t>::field; + return &aliased_field>::field; } #endif @@ -104,14 +102,13 @@ namespace sqlite_orm { constexpr decltype(auto) find_column_name(const DBOs& dboObjects, const column_pointer>&) { using table_type = storage_pick_table_t; - using cte_mapper_type = cte_mapper_type_t; + using cte_colrefs_tuple = typename cte_mapper_type_t::final_colrefs_tuple; using column_index_sequence = filter_tuple_sequence_t, is_column>; // note: even though the columns contain the [`aliased_field<>::*`] we perform the lookup using the column references. // lookup ColAlias in the final column references - using colalias_index = - find_tuple_type>; - static_assert(colalias_index::value < std::tuple_size_v, + using colalias_index = find_tuple_type>; + static_assert(colalias_index::value < std::tuple_size_v, "No such column mapped into the CTE."); // note: we could "materialize" the alias to an `aliased_field<>::*` and use the regular `table_t<>::find_column_name()` mechanism; diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 04a32b3ee..01557d002 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -12704,17 +12704,15 @@ namespace sqlite_orm { constexpr decltype(auto) materialize_column_pointer(const DBOs&, const column_pointer>&) { using table_type = storage_pick_table_t; - using cte_mapper_type = cte_mapper_type_t; + using cte_colrefs_tuple = typename cte_mapper_type_t::final_colrefs_tuple; + using cte_fields_type = typename cte_mapper_type_t::fields_type; // lookup ColAlias in the final column references - using colalias_index = - find_tuple_type>; - static_assert(colalias_index::value < std::tuple_size_v, + using colalias_index = find_tuple_type>; + static_assert(colalias_index::value < std::tuple_size_v, "No such column mapped into the CTE."); - return &aliased_field< - ColAlias, - std::tuple_element_t>::field; + return &aliased_field>::field; } #endif @@ -12738,14 +12736,13 @@ namespace sqlite_orm { constexpr decltype(auto) find_column_name(const DBOs& dboObjects, const column_pointer>&) { using table_type = storage_pick_table_t; - using cte_mapper_type = cte_mapper_type_t; + using cte_colrefs_tuple = typename cte_mapper_type_t::final_colrefs_tuple; using column_index_sequence = filter_tuple_sequence_t, is_column>; // note: even though the columns contain the [`aliased_field<>::*`] we perform the lookup using the column references. // lookup ColAlias in the final column references - using colalias_index = - find_tuple_type>; - static_assert(colalias_index::value < std::tuple_size_v, + using colalias_index = find_tuple_type>; + static_assert(colalias_index::value < std::tuple_size_v, "No such column mapped into the CTE."); // note: we could "materialize" the alias to an `aliased_field<>::*` and use the regular `table_t<>::find_column_name()` mechanism; @@ -13981,15 +13978,15 @@ namespace sqlite_orm { std::binary_semaphore& sync; }; - connection_holder(std::string filename, bool openedForeverHint, std::function onAfterOpen) : - _openedForeverHint{openedForeverHint}, _onAfterOpen{std::move(onAfterOpen)}, - filename(std::move(filename)) {} + connection_holder(std::string filename, bool openedForeverHint, std::function didOpenDb) : + _openedForeverHint{openedForeverHint}, _didOpenDb{std::move(didOpenDb)}, filename(std::move(filename)) { + } connection_holder(const connection_holder&) = delete; - connection_holder(const connection_holder& other, std::function onAfterOpen) : - filename{other.filename}, _openedForeverHint{other._openedForeverHint}, - _onAfterOpen{std::move(onAfterOpen)} {} + connection_holder(const connection_holder& other, std::function didOpenDb) : + _openedForeverHint{other._openedForeverHint}, _didOpenDb{std::move(didOpenDb)}, + filename{other.filename} {} void retain() { const maybe_lock maybeLock{_sync, !_openedForeverHint}; @@ -14011,8 +14008,8 @@ namespace sqlite_orm { throw_translated_sqlite_error(this->db); } - if (_onAfterOpen) { - _onAfterOpen(this->db); + if (_didOpenDb) { + _didOpenDb(this->db); } } @@ -14032,7 +14029,7 @@ namespace sqlite_orm { // last one closes the connection. - if (int rc = sqlite3_close(this->db); rc != SQLITE_OK) [[unlikely]] { + if (int rc = sqlite3_close_v2(this->db); rc != SQLITE_OK) [[unlikely]] { throw_translated_sqlite_error(this->db); } else { this->db = nullptr; @@ -14060,8 +14057,7 @@ namespace sqlite_orm { std::binary_semaphore _sync{1}; private: - alignas( - polyfill::hardware_destructive_interference_size) const std::function _onAfterOpen; + alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; public: const std::string filename; @@ -14070,13 +14066,13 @@ namespace sqlite_orm { struct connection_holder { connection_holder(std::string filename, bool /*openedForeverHint*/, - std::function onAfterOpen) : - _onAfterOpen{std::move(onAfterOpen)}, filename(std::move(filename)) {} + std::function didOpenDb) : + _didOpenDb{std::move(didOpenDb)}, filename(std::move(filename)) {} connection_holder(const connection_holder&) = delete; - connection_holder(const connection_holder& other, std::function onAfterOpen) : - _onAfterOpen{std::move(onAfterOpen)}, filename{other.filename} {} + connection_holder(const connection_holder& other, std::function didOpenDb) : + _didOpenDb{std::move(didOpenDb)}, filename{other.filename} {} void retain() { // first one opens the connection. @@ -14090,10 +14086,10 @@ namespace sqlite_orm { if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); } - } - if (_onAfterOpen) { - _onAfterOpen(this->db); + if (_didOpenDb) { + _didOpenDb(this->db); + } } } @@ -14101,7 +14097,7 @@ namespace sqlite_orm { // last one closes the connection. // we assume that this might happen by any thread, therefore the counter must serve as a synchronization point. if (_retainCount.fetch_sub(1, std::memory_order_acq_rel) == 1) { - int rc = sqlite3_close(this->db); + int rc = sqlite3_close_v2(this->db); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY { throw_translated_sqlite_error(this->db); } else { @@ -14135,7 +14131,7 @@ namespace sqlite_orm { #ifdef SQLITE_ORM_ALIGNED_NEW_SUPPORTED alignas(polyfill::hardware_destructive_interference_size) #endif - const std::function _onAfterOpen; + const std::function _didOpenDb; public: const std::string filename; @@ -24362,7 +24358,11 @@ namespace sqlite_orm { SQLITE_ORM_EXPORT namespace sqlite_orm { #ifdef SQLITE_ORM_CTAD_SUPPORTED /* - * Factory function for a storage, from a database file and a bunch of database object definitions. + * Factory function for a storage instance, from a database file, a set of database object definitions + * and option storage options like connection control options and an 'on open' callback. + * + * E.g. + * auto storage = make_storage("", connection_control{.open_forever = true}, on_open([](sqlite3* db) {})); */ template auto make_storage(std::string filename, Spec... specifications) { diff --git a/tests/storage_tests.cpp b/tests/storage_tests.cpp index 0a3598641..c3844ab53 100644 --- a/tests/storage_tests.cpp +++ b/tests/storage_tests.cpp @@ -9,39 +9,51 @@ TEST_CASE("connection control") { const auto openForever = GENERATE(false, true); SECTION("") { bool onOpenCalled = false; + int nOnOpenCalled = 0; SECTION("empty") { - auto storage = make_storage("", connection_control{openForever}, on_open([&onOpenCalled](sqlite3* db) { - onOpenCalled = db || false; - })); + auto storage = + make_storage("", connection_control{openForever}, on_open([&onOpenCalled, &nOnOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + ++nOnOpenCalled; + })); REQUIRE(storage.is_opened()); REQUIRE(onOpenCalled); + REQUIRE(nOnOpenCalled == 1); } #if __cpp_designated_initializers >= 201707L SECTION("empty C++20") { - auto storage = - make_storage("", connection_control{.open_forever = openForever}, on_open([&onOpenCalled](sqlite3* db) { - onOpenCalled = db || false; - })); + auto storage = make_storage("", + connection_control{.open_forever = openForever}, + on_open([&onOpenCalled, &nOnOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + ++nOnOpenCalled; + })); REQUIRE(storage.is_opened()); REQUIRE(onOpenCalled); + REQUIRE(nOnOpenCalled == 1); } #endif SECTION("memory") { - auto storage = - make_storage(":memory:", connection_control{openForever}, on_open([&onOpenCalled](sqlite3* db) { - onOpenCalled = db || false; - })); + auto storage = make_storage(":memory:", + connection_control{openForever}, + on_open([&onOpenCalled, &nOnOpenCalled](sqlite3* db) { + onOpenCalled = db || false; + ++nOnOpenCalled; + })); REQUIRE(storage.is_opened()); REQUIRE(onOpenCalled); + REQUIRE(nOnOpenCalled == 1); } SECTION("file name") { auto storage = make_storage("myDatabase.sqlite", connection_control{openForever}, - on_open([&onOpenCalled](sqlite3* db) { + on_open([&onOpenCalled, &nOnOpenCalled](sqlite3* db) { onOpenCalled = db || false; + ++nOnOpenCalled; })); REQUIRE(storage.is_opened() == openForever); REQUIRE(onOpenCalled == openForever); + REQUIRE(nOnOpenCalled == int(openForever)); } } } From e186657f3ad62e654f677fff9e376a4642aa44e3 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Fri, 21 Mar 2025 16:59:21 +0200 Subject: [PATCH 13/18] Maximum efficient result row stepping loop with `goto` --- dev/util.h | 24 ++++++++++++------------ include/sqlite_orm/sqlite_orm.h | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/dev/util.h b/dev/util.h index cff59dfc5..c60d0176f 100644 --- a/dev/util.h +++ b/dev/util.h @@ -116,19 +116,19 @@ namespace sqlite_orm { template void perform_steps(sqlite3_stmt* stmt, L&& lambda) { - int rc; - do { - switch (rc = sqlite3_step(stmt)) { - case SQLITE_ROW: { - lambda(stmt); - } break; - case SQLITE_DONE: - break; - default: { - throw_translated_sqlite_error(stmt); - } + // note: maximum efficient loop with `goto` + step: + switch (int rc = sqlite3_step(stmt)) { + case SQLITE_ROW: { + lambda(stmt); } - } while (rc != SQLITE_DONE); + goto step; + case SQLITE_DONE: + break; + default: { + throw_translated_sqlite_error(stmt); + } + } } } } diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 2b520fffc..7038e07a8 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13842,19 +13842,19 @@ namespace sqlite_orm { template void perform_steps(sqlite3_stmt* stmt, L&& lambda) { - int rc; - do { - switch (rc = sqlite3_step(stmt)) { - case SQLITE_ROW: { - lambda(stmt); - } break; - case SQLITE_DONE: - break; - default: { - throw_translated_sqlite_error(stmt); - } + // note: maximum efficient loop with `goto` + step: + switch (int rc = sqlite3_step(stmt)) { + case SQLITE_ROW: { + lambda(stmt); } - } while (rc != SQLITE_DONE); + goto step; + case SQLITE_DONE: + break; + default: { + throw_translated_sqlite_error(stmt); + } + } } } } From 4adb678d18c239b8680ceb69c7aedab6f65a8979 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Fri, 21 Mar 2025 17:46:09 +0200 Subject: [PATCH 14/18] Enabled SQLite extended error codes --- dev/connection_holder.h | 14 +++++++++-- dev/error_code.h | 4 ++-- dev/storage.h | 4 ++-- include/sqlite_orm/sqlite_orm.h | 22 +++++++++++++----- tests/constraints/unique.cpp | 14 +++++++---- tests/prepared_statement_tests/get.cpp | 7 +++--- .../insert_explicit.cpp | 9 ++++++-- .../replace_range.cpp | 18 ++++++++------- tests/schema/index_tests.cpp | 9 ++++++-- tests/storage_non_crud_tests.cpp | 17 ++++++++++---- tests/tests.cpp | 23 +++++++++++++------ tests/transaction_tests.cpp | 8 +++---- tests/user_defined_functions.cpp | 5 ++-- 13 files changed, 105 insertions(+), 49 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index c89bd4780..213d48bc2 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -68,7 +68,12 @@ namespace sqlite_orm { if (int rc = sqlite3_open_v2(this->filename.c_str(), &this->db, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE +#if SQLITE_VERSION_NUMBER >= 3008008 + | SQLITE_OPEN_EXRESCODE, +#else + | 0, +#endif nullptr); rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); @@ -147,7 +152,12 @@ namespace sqlite_orm { if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open_v2(this->filename.c_str(), &this->db, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE +#if SQLITE_VERSION_NUMBER >= 3008008 + | SQLITE_OPEN_EXRESCODE, +#else + | 0, +#endif nullptr); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); diff --git a/dev/error_code.h b/dev/error_code.h index 4830b85bd..a473c780f 100644 --- a/dev/error_code.h +++ b/dev/error_code.h @@ -112,8 +112,8 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { return "SQLite error"; } - std::string message(int c) const override final { - return sqlite3_errstr(c); + std::string message(int ev) const override final { + return sqlite3_errstr(ev); } }; diff --git a/dev/storage.h b/dev/storage.h index e86410487..381722290 100644 --- a/dev/storage.h +++ b/dev/storage.h @@ -1604,8 +1604,8 @@ namespace sqlite_orm { return std::move(res).value(); #else auto& table = this->get_table(); - auto stepRes = sqlite3_step(stmt); - switch (stepRes) { + int rc = sqlite3_step(stmt); + switch (rc) { case SQLITE_ROW: { T res; object_from_column_builder builder{res, stmt}; diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 7038e07a8..9a9db0002 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -3078,8 +3078,8 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { return "SQLite error"; } - std::string message(int c) const override final { - return sqlite3_errstr(c); + std::string message(int ev) const override final { + return sqlite3_errstr(ev); } }; @@ -14095,7 +14095,12 @@ namespace sqlite_orm { if (int rc = sqlite3_open_v2(this->filename.c_str(), &this->db, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE +#if SQLITE_VERSION_NUMBER >= 3008008 + | SQLITE_OPEN_EXRESCODE, +#else + | 0, +#endif nullptr); rc != SQLITE_OK) [[unlikely]] /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); @@ -14174,7 +14179,12 @@ namespace sqlite_orm { if (_retainCount.fetch_add(1, std::memory_order_relaxed) == 0) { int rc = sqlite3_open_v2(this->filename.c_str(), &this->db, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, + SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE +#if SQLITE_VERSION_NUMBER >= 3008008 + | SQLITE_OPEN_EXRESCODE, +#else + | 0, +#endif nullptr); if (rc != SQLITE_OK) SQLITE_ORM_CPP_UNLIKELY /*possible, but unexpected*/ { throw_translated_sqlite_error(this->db); @@ -24326,8 +24336,8 @@ namespace sqlite_orm { return std::move(res).value(); #else auto& table = this->get_table(); - auto stepRes = sqlite3_step(stmt); - switch (stepRes) { + int rc = sqlite3_step(stmt); + switch (rc) { case SQLITE_ROW: { T res; object_from_column_builder builder{res, stmt}; diff --git a/tests/constraints/unique.cpp b/tests/constraints/unique.cpp index bea9d37c2..4d9859bd3 100644 --- a/tests/constraints/unique.cpp +++ b/tests/constraints/unique.cpp @@ -1,10 +1,15 @@ #include #include +#include "../catch_matchers.h" using namespace sqlite_orm; TEST_CASE("Unique") { - using Catch::Matchers::ContainsSubstring; +#if SQLITE_VERSION_NUMBER >= 3008008 + const ErrorCodeExceptionMatcher uniqueExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT_UNIQUE)); +#else + const ErrorCodeExceptionMatcher uniqueExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT)); +#endif struct Contact { int id = 0; @@ -39,12 +44,13 @@ TEST_CASE("Unique") { storage.insert(Contact{0, "John", "Doe", "john.doe@gmail.com"}); - REQUIRE_THROWS_WITH(storage.insert(Contact{0, "Johnny", "Doe", "john.doe@gmail.com"}), - ContainsSubstring("constraint failed")); + REQUIRE_THROWS_MATCHES(storage.insert(Contact{0, "Johnny", "Doe", "john.doe@gmail.com"}), + std::system_error, + uniqueExceptionMatcher); storage.insert(Shape{0, "red", "green"}); storage.insert(Shape{0, "red", "blue"}); - REQUIRE_THROWS_WITH(storage.insert(Shape{0, "red", "green"}), ContainsSubstring("constraint failed")); + REQUIRE_THROWS_MATCHES(storage.insert(Shape{0, "red", "green"}), std::system_error, uniqueExceptionMatcher); std::vector lists(2); REQUIRE_NOTHROW(storage.insert_range(lists.begin(), lists.end())); diff --git a/tests/prepared_statement_tests/get.cpp b/tests/prepared_statement_tests/get.cpp index bad7422fd..fb863764c 100644 --- a/tests/prepared_statement_tests/get.cpp +++ b/tests/prepared_statement_tests/get.cpp @@ -1,5 +1,6 @@ #include #include +#include "../catch_matchers.h" #include "prepared_common.h" @@ -8,8 +9,8 @@ using namespace sqlite_orm; TEST_CASE("Prepared get") { using namespace PreparedStatementTests; - using Catch::Matchers::ContainsSubstring; using Catch::Matchers::UnorderedEquals; + const ErrorCodeExceptionMatcher notFoundExceptionMatcher(orm_error_code::not_found); const int defaultVisitTime = 50; @@ -73,7 +74,7 @@ TEST_CASE("Prepared get") { } { get<0>(statement) = 4; - REQUIRE_THROWS_WITH(storage.execute(statement), ContainsSubstring("Not found")); + REQUIRE_THROWS_MATCHES(storage.execute(statement), std::system_error, notFoundExceptionMatcher); } } } @@ -95,7 +96,7 @@ TEST_CASE("Prepared get") { //.. } SECTION("execute") { - REQUIRE_THROWS_WITH(storage.execute(statement), ContainsSubstring("Not found")); + REQUIRE_THROWS_MATCHES(storage.execute(statement), std::system_error, notFoundExceptionMatcher); } } { diff --git a/tests/prepared_statement_tests/insert_explicit.cpp b/tests/prepared_statement_tests/insert_explicit.cpp index b4056e642..7b32cbbeb 100644 --- a/tests/prepared_statement_tests/insert_explicit.cpp +++ b/tests/prepared_statement_tests/insert_explicit.cpp @@ -1,5 +1,6 @@ #include #include +#include "../catch_matchers.h" #include "prepared_common.h" @@ -8,8 +9,12 @@ using namespace sqlite_orm; TEST_CASE("Prepared insert explicit") { using namespace PreparedStatementTests; - using Catch::Matchers::ContainsSubstring; using Catch::Matchers::UnorderedEquals; +#if SQLITE_VERSION_NUMBER >= 3008008 + const ErrorCodeExceptionMatcher primaryKeyExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT_PRIMARYKEY)); +#else + const ErrorCodeExceptionMatcher primaryKeyExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT)); +#endif const int defaultVisitTime = 50; @@ -68,7 +73,7 @@ TEST_CASE("Prepared insert explicit") { { user.id = 6; user.name = "Nate Dogg"; - REQUIRE_THROWS_WITH(storage.execute(statement), ContainsSubstring("constraint failed")); + REQUIRE_THROWS_MATCHES(storage.execute(statement), std::system_error, primaryKeyExceptionMatcher); get<0>(statement) = user; auto insertedId = storage.execute(statement); diff --git a/tests/prepared_statement_tests/replace_range.cpp b/tests/prepared_statement_tests/replace_range.cpp index 022a2bf44..95f1ed66d 100644 --- a/tests/prepared_statement_tests/replace_range.cpp +++ b/tests/prepared_statement_tests/replace_range.cpp @@ -41,9 +41,6 @@ TEST_CASE("Prepared replace range") { std::vector users; std::vector> userPointers; std::vector expected; - auto lambda = [](const std::unique_ptr& pointer) -> const User& { - return *pointer; - }; SECTION("empty") { using Catch::Matchers::ContainsSubstring; @@ -55,8 +52,10 @@ TEST_CASE("Prepared replace range") { ContainsSubstring("incomplete input")); } SECTION("pointers") { - REQUIRE_THROWS_WITH(storage.prepare(replace_range(userPointers.begin(), userPointers.end(), lambda)), - ContainsSubstring("incomplete input")); + REQUIRE_THROWS_WITH( + storage.prepare( + replace_range(userPointers.begin(), userPointers.end(), &std::unique_ptr::operator*)), + ContainsSubstring("incomplete input")); } } SECTION("one existing") { @@ -73,7 +72,8 @@ TEST_CASE("Prepared replace range") { } SECTION("pointers") { userPointers.push_back(std::make_unique(user)); - auto statement = storage.prepare(replace_range(userPointers.begin(), userPointers.end(), lambda)); + auto statement = storage.prepare( + replace_range(userPointers.begin(), userPointers.end(), &std::unique_ptr::operator*)); REQUIRE(get<0>(statement) == userPointers.begin()); REQUIRE(get<1>(statement) == userPointers.end()); storage.execute(statement); @@ -97,7 +97,8 @@ TEST_CASE("Prepared replace range") { SECTION("pointers") { userPointers.push_back(std::make_unique(user)); userPointers.push_back(std::make_unique(user2)); - auto statement = storage.prepare(replace_range(userPointers.begin(), userPointers.end(), lambda)); + auto statement = storage.prepare( + replace_range(userPointers.begin(), userPointers.end(), &std::unique_ptr::operator*)); REQUIRE(get<0>(statement) == userPointers.begin()); REQUIRE(get<1>(statement) == userPointers.end()); storage.execute(statement); @@ -121,7 +122,8 @@ TEST_CASE("Prepared replace range") { userPointers.push_back(std::make_unique(user)); userPointers.push_back(std::make_unique(user2)); userPointers.push_back(std::make_unique(user3)); - auto statement = storage.prepare(replace_range(userPointers.begin(), userPointers.end(), lambda)); + auto statement = storage.prepare( + replace_range(userPointers.begin(), userPointers.end(), &std::unique_ptr::operator*)); REQUIRE(get<0>(statement) == userPointers.begin()); REQUIRE(get<1>(statement) == userPointers.end()); storage.execute(statement); diff --git a/tests/schema/index_tests.cpp b/tests/schema/index_tests.cpp index 01328b3f1..b672596d5 100644 --- a/tests/schema/index_tests.cpp +++ b/tests/schema/index_tests.cpp @@ -1,5 +1,6 @@ #include #include +#include "../catch_matchers.h" using namespace sqlite_orm; @@ -66,7 +67,11 @@ TEST_CASE("index") { #ifdef SQLITE_ORM_OPTIONAL_SUPPORTED TEST_CASE("filtered index") { - using Catch::Matchers::ContainsSubstring; +#if SQLITE_VERSION_NUMBER >= 3008008 + const ErrorCodeExceptionMatcher uniqueExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT_UNIQUE)); +#else + const ErrorCodeExceptionMatcher uniqueExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT)); +#endif struct Test { std::optional field1 = 0; @@ -80,7 +85,7 @@ TEST_CASE("filtered index") { REQUIRE_NOTHROW(storage.sync_schema()); storage.insert(Test{1, std::nullopt}); - REQUIRE_THROWS_WITH(storage.insert(Test{1, std::nullopt}), ContainsSubstring("constraint failed")); + REQUIRE_THROWS_MATCHES(storage.insert(Test{1, std::nullopt}), std::system_error, uniqueExceptionMatcher); } SECTION("2") { auto storage = make_storage( diff --git a/tests/storage_non_crud_tests.cpp b/tests/storage_non_crud_tests.cpp index 54f5e5dd2..c4b275860 100644 --- a/tests/storage_non_crud_tests.cpp +++ b/tests/storage_non_crud_tests.cpp @@ -1,6 +1,7 @@ #include #include #include +#include "catch_matchers.h" using namespace sqlite_orm; @@ -503,7 +504,11 @@ TEST_CASE("Remove all") { } TEST_CASE("Explicit insert") { - using Catch::Matchers::ContainsSubstring; +#if SQLITE_VERSION_NUMBER >= 3008008 + const ErrorCodeExceptionMatcher notNullExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT_NOTNULL)); +#else + const ErrorCodeExceptionMatcher notNullExceptionMatcher(sqlite_errc(SQLITE_CONSTRAINT)); +#endif struct User { int id; @@ -592,8 +597,9 @@ TEST_CASE("Explicit insert") { SECTION("one column") { User user4; user4.name = "Egor"; - REQUIRE_THROWS_WITH(storage.insert(user4, columns(&User::name)), - ContainsSubstring("NOT NULL constraint failed")); + REQUIRE_THROWS_MATCHES(storage.insert(user4, columns(&User::name)), + std::system_error, + notNullExceptionMatcher); } } SECTION("visit") { @@ -625,8 +631,9 @@ TEST_CASE("Explicit insert") { Visit visit3; visit3.setId(10); SECTION("getter") { - REQUIRE_THROWS_WITH(storage.insert(visit3, columns(&Visit::id)), - ContainsSubstring("NOT NULL constraint failed")); + REQUIRE_THROWS_MATCHES(storage.insert(visit3, columns(&Visit::id)), + std::system_error, + notNullExceptionMatcher); } } } diff --git a/tests/tests.cpp b/tests/tests.cpp index 4b354657f..beb6e1edb 100644 --- a/tests/tests.cpp +++ b/tests/tests.cpp @@ -1,5 +1,6 @@ #include #include +#include "catch_matchers.h" #include // std::vector #include // std::string @@ -101,7 +102,11 @@ TEST_CASE("Limits") { } TEST_CASE("Custom collate") { - using Catch::Matchers::ContainsSubstring; +#if SQLITE_VERSION_NUMBER >= 3008008 + const ErrorCodeExceptionMatcher collSequExceptionMatcher(sqlite_errc(SQLITE_ERROR_MISSING_COLLSEQ)); +#else + const ErrorCodeExceptionMatcher collSequExceptionMatcher(sqlite_errc(SQLITE_ERROR)); +#endif struct Item { int id; @@ -186,12 +191,16 @@ TEST_CASE("Custom collate") { } else { storage.delete_collation(); } - REQUIRE_THROWS_WITH(storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate("ototo"))), - ContainsSubstring("no such collation sequence")); - REQUIRE_THROWS_WITH(storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate())), - ContainsSubstring("no such collation sequence")); - REQUIRE_THROWS_WITH(storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate("ototo2"))), - ContainsSubstring("no such collation sequence")); + REQUIRE_THROWS_MATCHES(storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate("ototo"))), + std::system_error, + collSequExceptionMatcher); + REQUIRE_THROWS_MATCHES( + storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate())), + std::system_error, + collSequExceptionMatcher); + REQUIRE_THROWS_MATCHES(storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate("ototo2"))), + std::system_error, + collSequExceptionMatcher); rows = storage.select(&Item::name, where(is_equal(&Item::name, "Mercury").collate("alwaysequal")), diff --git a/tests/transaction_tests.cpp b/tests/transaction_tests.cpp index 129c08146..5654a5c8e 100644 --- a/tests/transaction_tests.cpp +++ b/tests/transaction_tests.cpp @@ -64,6 +64,9 @@ TEST_CASE("begin_transaction") { } TEST_CASE("Transaction guard") { + const ErrorCodeExceptionMatcher notFoundExceptionMatcher(orm_error_code::not_found); + const ErrorCodeExceptionMatcher busyExceptionMatcher(sqlite_errc(SQLITE_BUSY)); + std::remove("guard.sqlite"); auto table = make_table("objects", make_column("id", &Object::id, primary_key()), make_column("name", &Object::name)); @@ -74,7 +77,6 @@ TEST_CASE("Transaction guard") { storage.insert(Object{0, "Jack"}); - const ErrorCodeExceptionMatcher notFoundExceptionMatcher(orm_error_code::not_found); SECTION("insert, call make a storage to call an exception and check that rollback was fired") { auto countBefore = storage.count(); SECTION("transaction_guard") { @@ -329,8 +331,6 @@ TEST_CASE("Transaction guard") { REQUIRE(storage.count() == countBefore); } SECTION("exception propagated from dtor") { - using Catch::Matchers::ContainsSubstring; - // create a second database connection auto storage2 = make_storage("guard.sqlite", table); auto guard2 = storage2.transaction_guard(); @@ -340,7 +340,7 @@ TEST_CASE("Transaction guard") { auto guard = new (&buffer) internal::transaction_guard_t{storage.transaction_guard()}; storage.insert({}); guard->commit_on_destroy = true; - REQUIRE_THROWS_WITH(guard->~transaction_guard_t(), ContainsSubstring("database is locked")); + REQUIRE_THROWS_MATCHES(guard->~transaction_guard_t(), std::system_error, busyExceptionMatcher); } std::remove("guard.sqlite"); } diff --git a/tests/user_defined_functions.cpp b/tests/user_defined_functions.cpp index 20e34653d..c53c4d268 100644 --- a/tests/user_defined_functions.cpp +++ b/tests/user_defined_functions.cpp @@ -280,6 +280,7 @@ struct NonDefaultCtorAggregateFunction { TEST_CASE("custom functions") { using Catch::Matchers::ContainsSubstring; + const ErrorCodeExceptionMatcher noMemExceptionMatcher(sqlite_errc(SQLITE_NOMEM)); SqrtFunction::callsCount = 0; StatelessHasPrefixFunction::callsCount = 0; @@ -309,7 +310,7 @@ TEST_CASE("custom functions") { // test w/o a result set, i.e when the final aggregate call is the first to require the aggregate function REQUIRE_THROWS_MATCHES(storage.select(func(&User::id)), std::system_error, - ErrorCodeExceptionMatcher(sqlite_errc(SQLITE_NOMEM))); + noMemExceptionMatcher); storage.delete_aggregate_function(); // call before creation @@ -425,7 +426,7 @@ TEST_CASE("custom functions") { storage.create_aggregate_function(); REQUIRE_THROWS_MATCHES(storage.select(func(&User::id)), std::system_error, - ErrorCodeExceptionMatcher(sqlite_errc(SQLITE_NOMEM))); + noMemExceptionMatcher); storage.delete_aggregate_function(); storage.create_scalar_function(); From 4df56f1a68c34d6bd2d17e2608c910296d346a16 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sat, 22 Mar 2025 22:10:08 +0200 Subject: [PATCH 15/18] Maximum efficient result row stepping loop --- dev/util.h | 23 +++++++++++------------ include/sqlite_orm/sqlite_orm.h | 23 +++++++++++------------ 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/dev/util.h b/dev/util.h index c60d0176f..9bd419e4d 100644 --- a/dev/util.h +++ b/dev/util.h @@ -107,7 +107,7 @@ namespace sqlite_orm { lambda(stmt); } break; case SQLITE_DONE: - break; + return; default: { throw_translated_sqlite_error(stmt); } @@ -116,17 +116,16 @@ namespace sqlite_orm { template void perform_steps(sqlite3_stmt* stmt, L&& lambda) { - // note: maximum efficient loop with `goto` - step: - switch (int rc = sqlite3_step(stmt)) { - case SQLITE_ROW: { - lambda(stmt); - } - goto step; - case SQLITE_DONE: - break; - default: { - throw_translated_sqlite_error(stmt); + for (;;) { + switch (int rc = sqlite3_step(stmt)) { + case SQLITE_ROW: { + lambda(stmt); + } break; + case SQLITE_DONE: + return; + default: { + throw_translated_sqlite_error(stmt); + } } } } diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 9a9db0002..2fc2d1236 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13833,7 +13833,7 @@ namespace sqlite_orm { lambda(stmt); } break; case SQLITE_DONE: - break; + return; default: { throw_translated_sqlite_error(stmt); } @@ -13842,17 +13842,16 @@ namespace sqlite_orm { template void perform_steps(sqlite3_stmt* stmt, L&& lambda) { - // note: maximum efficient loop with `goto` - step: - switch (int rc = sqlite3_step(stmt)) { - case SQLITE_ROW: { - lambda(stmt); - } - goto step; - case SQLITE_DONE: - break; - default: { - throw_translated_sqlite_error(stmt); + for (;;) { + switch (int rc = sqlite3_step(stmt)) { + case SQLITE_ROW: { + lambda(stmt); + } break; + case SQLITE_DONE: + return; + default: { + throw_translated_sqlite_error(stmt); + } } } } From 4ff1aff21311fcd4397fb36f6b8ae526eb6d7078 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sat, 22 Mar 2025 22:11:32 +0200 Subject: [PATCH 16/18] Removed superfluous value cast to char --- dev/util.h | 2 +- include/sqlite_orm/sqlite_orm.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/util.h b/dev/util.h index 9bd419e4d..eec8e2844 100644 --- a/dev/util.h +++ b/dev/util.h @@ -38,7 +38,7 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { */ inline std::string quote_blob_literal(std::string v) { constexpr char quoteChar = '\''; - return std::string{char('x'), quoteChar} + std::move(v) + quoteChar; + return std::string{'x', quoteChar} + std::move(v) + quoteChar; } /** diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index 2fc2d1236..1392956c6 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -13764,7 +13764,7 @@ SQLITE_ORM_EXPORT namespace sqlite_orm { */ inline std::string quote_blob_literal(std::string v) { constexpr char quoteChar = '\''; - return std::string{char('x'), quoteChar} + std::move(v) + quoteChar; + return std::string{'x', quoteChar} + std::move(v) + quoteChar; } /** From 815bec4e9e95294db20d0aa40b084d65b21cd4bd Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Sun, 23 Mar 2025 08:57:07 +0200 Subject: [PATCH 17/18] Used C API for strings from `std` namespace in unit tests --- tests/prepared_statement_tests/insert.cpp | 7 ++++--- tests/prepared_statement_tests/replace.cpp | 7 ++++--- tests/storage_non_crud_tests.cpp | 7 ++++--- tests/tests.cpp | 5 +++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/prepared_statement_tests/insert.cpp b/tests/prepared_statement_tests/insert.cpp index 6dbbc1f8c..c1b3808d6 100644 --- a/tests/prepared_statement_tests/insert.cpp +++ b/tests/prepared_statement_tests/insert.cpp @@ -1,5 +1,6 @@ #include #include +#include // std::strcmp #include "prepared_common.h" @@ -76,7 +77,7 @@ TEST_CASE("Prepared insert") { insert(into(), columns(&User::id, &User::name), values(std::make_tuple(1, "Ellie")))); storage.execute(statement); REQUIRE(get<0>(statement) == 1); - REQUIRE(::strcmp(get<1>(statement), "Ellie") == 0); + REQUIRE(std::strcmp(get<1>(statement), "Ellie") == 0); } SECTION("no statement") { storage.insert(into(), columns(&User::id, &User::name), values(std::make_tuple(1, "Ellie"))); @@ -93,9 +94,9 @@ TEST_CASE("Prepared insert") { values(std::make_tuple(1, "Ellie"), std::make_tuple(5, "Calvin")))); storage.execute(statement); REQUIRE(get<0>(statement) == 1); - REQUIRE(::strcmp(get<1>(statement), "Ellie") == 0); + REQUIRE(std::strcmp(get<1>(statement), "Ellie") == 0); REQUIRE(get<2>(statement) == 5); - REQUIRE(::strcmp(get<3>(statement), "Calvin") == 0); + REQUIRE(std::strcmp(get<3>(statement), "Calvin") == 0); } SECTION("no statement") { storage.insert(into(), diff --git a/tests/prepared_statement_tests/replace.cpp b/tests/prepared_statement_tests/replace.cpp index e91bd5542..de9658fda 100644 --- a/tests/prepared_statement_tests/replace.cpp +++ b/tests/prepared_statement_tests/replace.cpp @@ -1,5 +1,6 @@ #include #include +#include // std::strcmp #include "prepared_common.h" @@ -41,7 +42,7 @@ TEST_CASE("Prepared replace") { replace(into(), columns(&User::id, &User::name), values(std::make_tuple(1, "Ellie")))); storage.execute(statement); REQUIRE(get<0>(statement) == 1); - REQUIRE(::strcmp(get<1>(statement), "Ellie") == 0); + REQUIRE(std::strcmp(get<1>(statement), "Ellie") == 0); } SECTION("no statement") { storage.replace(into(), columns(&User::id, &User::name), values(std::make_tuple(1, "Ellie"))); @@ -56,9 +57,9 @@ TEST_CASE("Prepared replace") { values(std::make_tuple(1, "Ellie"), std::make_tuple(5, "Calvin")))); storage.execute(statement); REQUIRE(get<0>(statement) == 1); - REQUIRE(::strcmp(get<1>(statement), "Ellie") == 0); + REQUIRE(std::strcmp(get<1>(statement), "Ellie") == 0); REQUIRE(get<2>(statement) == 5); - REQUIRE(::strcmp(get<3>(statement), "Calvin") == 0); + REQUIRE(std::strcmp(get<3>(statement), "Calvin") == 0); } SECTION("no statement") { storage.replace(into(), diff --git a/tests/storage_non_crud_tests.cpp b/tests/storage_non_crud_tests.cpp index c4b275860..f0fc5f0f9 100644 --- a/tests/storage_non_crud_tests.cpp +++ b/tests/storage_non_crud_tests.cpp @@ -1,6 +1,7 @@ #include #include #include +#include // std::strcmp #include "catch_matchers.h" using namespace sqlite_orm; @@ -269,9 +270,9 @@ TEST_CASE("Select") { throw std::runtime_error(sqlite3_errmsg(db)); } REQUIRE(sqlite3_column_int(stmt, 0) == firstId); - REQUIRE(::strcmp((const char*)sqlite3_column_text(stmt, 1), "best") == 0); - REQUIRE(::strcmp((const char*)sqlite3_column_text(stmt, 2), "behaviour") == 0); - REQUIRE(::strcmp((const char*)sqlite3_column_text(stmt, 3), "hey") == 0); + REQUIRE(std::strcmp((const char*)sqlite3_column_text(stmt, 1), "best") == 0); + REQUIRE(std::strcmp((const char*)sqlite3_column_text(stmt, 2), "behaviour") == 0); + REQUIRE(std::strcmp((const char*)sqlite3_column_text(stmt, 3), "hey") == 0); REQUIRE(sqlite3_column_int(stmt, 4) == 5); sqlite3_finalize(stmt); } diff --git a/tests/tests.cpp b/tests/tests.cpp index beb6e1edb..c9bfef84f 100644 --- a/tests/tests.cpp +++ b/tests/tests.cpp @@ -5,6 +5,7 @@ #include // std::vector #include // std::string #include // std::remove +#include // std::strncmp using namespace sqlite_orm; @@ -116,7 +117,7 @@ TEST_CASE("Custom collate") { struct OtotoCollation { int operator()(int leftLength, const void* lhs, int rightLength, const void* rhs) const { if (leftLength == rightLength) { - return ::strncmp((const char*)lhs, (const char*)rhs, leftLength); + return std::strncmp((const char*)lhs, (const char*)rhs, leftLength); } else { return 1; } @@ -158,7 +159,7 @@ TEST_CASE("Custom collate") { if (useLegacyScript) { storage.create_collation("ototo", [](int leftLength, const void* lhs, int rightLength, const void* rhs) { if (leftLength == rightLength) { - return ::strncmp((const char*)lhs, (const char*)rhs, leftLength); + return std::strncmp((const char*)lhs, (const char*)rhs, leftLength); } else { return 1; } From 14dd6b4a75e194cf960528fb30065620b63d6394 Mon Sep 17 00:00:00 2001 From: klaus triendl Date: Tue, 26 Aug 2025 21:39:39 +0200 Subject: [PATCH 18/18] Suppressed warning about over aligned structs --- dev/connection_holder.h | 12 ++++++++---- dev/functional/cxx_compiler_quirks.h | 13 +++++++++++++ include/sqlite_orm/sqlite_orm.h | 25 +++++++++++++++++++++---- tests/user_defined_functions.cpp | 9 +++------ 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/dev/connection_holder.h b/dev/connection_holder.h index 8b4503d82..b9a0d84e9 100644 --- a/dev/connection_holder.h +++ b/dev/connection_holder.h @@ -122,7 +122,8 @@ namespace sqlite_orm { } protected: - alignas(polyfill::hardware_destructive_interference_size) orm_gsl::owner db = nullptr; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + orm_gsl::owner db = nullptr; private: std::atomic_int _retainCount{}; @@ -130,7 +131,8 @@ namespace sqlite_orm { std::binary_semaphore _sync{1}; private: - alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + const std::function _didOpenDb; public: const std::string filename; @@ -200,13 +202,15 @@ namespace sqlite_orm { } protected: - alignas(polyfill::hardware_destructive_interference_size) orm_gsl::owner db = nullptr; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + orm_gsl::owner db = nullptr; private: std::atomic_int _retainCount{}; private: - alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + const std::function _didOpenDb; public: const std::string filename; diff --git a/dev/functional/cxx_compiler_quirks.h b/dev/functional/cxx_compiler_quirks.h index e666ad36d..14e0b05db 100644 --- a/dev/functional/cxx_compiler_quirks.h +++ b/dev/functional/cxx_compiler_quirks.h @@ -20,6 +20,16 @@ #define SQLITE_ORM_CLANG_SUPPRESS(warnoption, ...) __VA_ARGS__ #endif +#if defined(_MSC_VER) && !defined(__clang__) +#define SQLITE_ORM_DO_PRAGMA(...) __pragma(__VA_ARGS__) +#endif + +#if defined(_MSC_VER) && !defined(__clang__) +#define SQLITE_ORM_MSVC_SUPPRESS(warncode, ...) SQLITE_ORM_DO_PRAGMA(warning(suppress : warncode)) +#else +#define SQLITE_ORM_MSVC_SUPPRESS(warcode, ...) __VA_ARGS__ +#endif + // clang has the bad habit of diagnosing missing brace-init-lists when constructing aggregates with base classes. // This is a false positive, since the C++ standard is quite clear that braces for nested or base objects may be omitted, // see https://en.cppreference.com/w/cpp/language/aggregate_initialization: @@ -30,6 +40,9 @@ // Because we know what we are doing, we suppress the diagnostic message #define SQLITE_ORM_CLANG_SUPPRESS_MISSING_BRACES(...) SQLITE_ORM_CLANG_SUPPRESS("-Wmissing-braces", __VA_ARGS__) +// msvc has the bad habit of diagnosing overalignment of types with an explicit alignment specifier. +#define SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(...) SQLITE_ORM_MSVC_SUPPRESS(4324, __VA_ARGS__) + #if defined(_MSC_VER) && (_MSC_VER < 1920) #define SQLITE_ORM_BROKEN_VARIADIC_PACK_EXPANSION // Type replacement may fail if an alias template has a non-type template parameter from a dependent expression in it, diff --git a/include/sqlite_orm/sqlite_orm.h b/include/sqlite_orm/sqlite_orm.h index eac772bf9..e7be7aab4 100644 --- a/include/sqlite_orm/sqlite_orm.h +++ b/include/sqlite_orm/sqlite_orm.h @@ -150,6 +150,16 @@ using std::nullptr_t; #define SQLITE_ORM_CLANG_SUPPRESS(warnoption, ...) __VA_ARGS__ #endif +#if defined(_MSC_VER) && !defined(__clang__) +#define SQLITE_ORM_DO_PRAGMA(...) __pragma(__VA_ARGS__) +#endif + +#if defined(_MSC_VER) && !defined(__clang__) +#define SQLITE_ORM_MSVC_SUPPRESS(warncode, ...) SQLITE_ORM_DO_PRAGMA(warning(suppress : warncode)) +#else +#define SQLITE_ORM_MSVC_SUPPRESS(warcode, ...) __VA_ARGS__ +#endif + // clang has the bad habit of diagnosing missing brace-init-lists when constructing aggregates with base classes. // This is a false positive, since the C++ standard is quite clear that braces for nested or base objects may be omitted, // see https://en.cppreference.com/w/cpp/language/aggregate_initialization: @@ -160,6 +170,9 @@ using std::nullptr_t; // Because we know what we are doing, we suppress the diagnostic message #define SQLITE_ORM_CLANG_SUPPRESS_MISSING_BRACES(...) SQLITE_ORM_CLANG_SUPPRESS("-Wmissing-braces", __VA_ARGS__) +// msvc has the bad habit of diagnosing overalignment of types with an explicit alignment specifier. +#define SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(...) SQLITE_ORM_MSVC_SUPPRESS(4324, __VA_ARGS__) + #if defined(_MSC_VER) && (_MSC_VER < 1920) #define SQLITE_ORM_BROKEN_VARIADIC_PACK_EXPANSION // Type replacement may fail if an alias template has a non-type template parameter from a dependent expression in it, @@ -14382,7 +14395,8 @@ namespace sqlite_orm { } protected: - alignas(polyfill::hardware_destructive_interference_size) orm_gsl::owner db = nullptr; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + orm_gsl::owner db = nullptr; private: std::atomic_int _retainCount{}; @@ -14390,7 +14404,8 @@ namespace sqlite_orm { std::binary_semaphore _sync{1}; private: - alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + const std::function _didOpenDb; public: const std::string filename; @@ -14460,13 +14475,15 @@ namespace sqlite_orm { } protected: - alignas(polyfill::hardware_destructive_interference_size) orm_gsl::owner db = nullptr; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + orm_gsl::owner db = nullptr; private: std::atomic_int _retainCount{}; private: - alignas(polyfill::hardware_destructive_interference_size) const std::function _didOpenDb; + SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(polyfill::hardware_destructive_interference_size)) + const std::function _didOpenDb; public: const std::string filename; diff --git a/tests/user_defined_functions.cpp b/tests/user_defined_functions.cpp index 006e79a62..c54a0b01f 100644 --- a/tests/user_defined_functions.cpp +++ b/tests/user_defined_functions.cpp @@ -190,8 +190,7 @@ int MultiSum::objectsCount = 0; int FirstFunction::objectsCount = 0; int FirstFunction::callsCount = 0; -#if __cpp_aligned_new >= 201606L -struct alignas(2 * __STDCPP_DEFAULT_NEW_ALIGNMENT__) OverAlignedScalarFunction { +struct SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(2 * __STDCPP_DEFAULT_NEW_ALIGNMENT__)) OverAlignedScalarFunction { int operator()(int arg) const { return arg; } @@ -201,7 +200,8 @@ struct alignas(2 * __STDCPP_DEFAULT_NEW_ALIGNMENT__) OverAlignedScalarFunction { } }; -struct alignas(2 * __STDCPP_DEFAULT_NEW_ALIGNMENT__) OverAlignedAggregateFunction { +struct SQLITE_ORM_MSVC_SUPPRESS_OVERALIGNMENT(alignas(2 * + __STDCPP_DEFAULT_NEW_ALIGNMENT__)) OverAlignedAggregateFunction { double sum = 0; void step(double arg) { @@ -215,7 +215,6 @@ struct alignas(2 * __STDCPP_DEFAULT_NEW_ALIGNMENT__) OverAlignedAggregateFunctio return "OVERALIGNED2"; } }; -#endif #ifdef SQLITE_ORM_STATIC_CALL_OPERATOR_SUPPORTED struct StaticCallOpFunction { @@ -486,14 +485,12 @@ TEST_CASE("custom functions") { } storage.delete_aggregate_function(); -#if __cpp_aligned_new >= 201606L { storage.create_scalar_function(); REQUIRE_NOTHROW(storage.delete_scalar_function()); storage.create_aggregate_function(); REQUIRE_NOTHROW(storage.delete_aggregate_function()); } -#endif #ifdef SQLITE_ORM_STATIC_CALL_OPERATOR_SUPPORTED storage.create_scalar_function();