From 57a314cc7e86540b0b02c85295ac15f220056686 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Sun, 21 Apr 2024 19:19:45 +0200 Subject: [PATCH 1/4] Sketch fix --- include/soci/row.h | 5 +++++ src/core/row.cpp | 4 ++++ tests/common-tests.h | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/soci/row.h b/include/soci/row.h index 07f7c3432..43e77cb99 100644 --- a/include/soci/row.h +++ b/include/soci/row.h @@ -97,6 +97,11 @@ class SOCI_DECL row T ret; type_conversion::move_from_base(baseVal, *indicators_.at(pos), ret); + + // Re-initialize the holder in order to be able to use this row object + // for binding to another data set + baseVal = T{}; + return ret; } diff --git a/src/core/row.cpp b/src/core/row.cpp index af4bdc745..9e381b032 100644 --- a/src/core/row.cpp +++ b/src/core/row.cpp @@ -119,5 +119,9 @@ blob row::move_as(std::size_t pos) const blob ret; type_conversion::move_from_base(baseVal, *indicators_.at(pos), ret); + + // Re-initialize blob object so it can be used in further queries + baseVal.initialize(session); + return ret; } diff --git a/tests/common-tests.h b/tests/common-tests.h index 139bd2ba6..504e53ea8 100644 --- a/tests/common-tests.h +++ b/tests/common-tests.h @@ -6875,7 +6875,8 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]") } SECTION("move_as") { - soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id)); + //soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id)); + soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id union all select b from soci_test where id=:id", soci::use(id, "id")); bool containedData = false; for (auto it = rowSet.begin(); it != rowSet.end(); ++it) { From 4c3c4682757e23924872c53f3b14b1c08560b849 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 1 May 2024 15:54:32 +0200 Subject: [PATCH 2/4] Implement blob re-initialization --- include/private/soci-trivial-blob-backend.h | 6 ++++++ include/soci/blob.h | 1 + include/soci/db2/soci-db2.h | 1 + include/soci/empty/soci-empty.h | 2 ++ include/soci/firebird/soci-firebird.h | 2 ++ include/soci/odbc/soci-odbc.h | 1 + include/soci/oracle/soci-oracle.h | 2 ++ include/soci/postgresql/soci-postgresql.h | 2 ++ include/soci/soci-backend.h | 4 ++++ src/backends/db2/blob.cpp | 5 +++++ src/backends/empty/blob.cpp | 5 +++++ src/backends/firebird/blob.cpp | 5 +++++ src/backends/mysql/blob.cpp | 4 ++-- src/backends/odbc/blob.cpp | 5 +++++ src/backends/oracle/blob.cpp | 5 +++++ src/backends/postgresql/blob.cpp | 5 +++++ src/backends/sqlite3/blob.cpp | 4 ++-- src/core/blob.cpp | 7 ++++++- src/core/row.cpp | 2 +- 19 files changed, 62 insertions(+), 6 deletions(-) diff --git a/include/private/soci-trivial-blob-backend.h b/include/private/soci-trivial-blob-backend.h index cc298bb88..acd9aa60a 100644 --- a/include/private/soci-trivial-blob-backend.h +++ b/include/private/soci-trivial-blob-backend.h @@ -2,6 +2,7 @@ #define SOCI_PRIVATE_SOCI_TRIVIAL_BLOB_BACKEND_H_INCLUDED #include "soci/soci-backend.h" +#include "soci/session.h" #include #include @@ -22,6 +23,8 @@ namespace details class trivial_blob_backend : public details::blob_backend { public: + trivial_blob_backend(details::session_backend &backend) : session_(backend) {} + std::size_t get_len() override { return buffer_.size(); } std::size_t read_from_start(void* buf, std::size_t toRead, @@ -71,7 +74,10 @@ class trivial_blob_backend : public details::blob_backend const std::uint8_t *get_buffer() const { return buffer_.data(); } + details::session_backend &get_session_backend() override { return session_; } + protected: + details::session_backend &session_; std::vector< std::uint8_t > buffer_; }; diff --git a/include/soci/blob.h b/include/soci/blob.h index 2b3d1dccf..baf3350ba 100644 --- a/include/soci/blob.h +++ b/include/soci/blob.h @@ -40,6 +40,7 @@ class SOCI_DECL blob // (Re)initializes this blob void initialize(session &s); + void initialize(details::blob_backend *backend); std::size_t get_len(); diff --git a/include/soci/db2/soci-db2.h b/include/soci/db2/soci-db2.h index 45191d406..366c39baa 100644 --- a/include/soci/db2/soci-db2.h +++ b/include/soci/db2/soci-db2.h @@ -233,6 +233,7 @@ struct db2_blob_backend : details::blob_backend std::size_t write_from_start(const void* buf, std::size_t toWrite, std::size_t offset = 0) override; std::size_t append(const void* buf, std::size_t toWrite) override; void trim(std::size_t newLen) override; + details::session_backend &get_session_backend() override; db2_session_backend& session_; }; diff --git a/include/soci/empty/soci-empty.h b/include/soci/empty/soci-empty.h index fec2495be..7a48a8d81 100644 --- a/include/soci/empty/soci-empty.h +++ b/include/soci/empty/soci-empty.h @@ -146,6 +146,8 @@ struct empty_blob_backend : details::blob_backend std::size_t append(const void* buf, std::size_t toWrite) override; void trim(std::size_t newLen) override; + details::session_backend &get_session_backend() override; + empty_session_backend& session_; }; diff --git a/include/soci/firebird/soci-firebird.h b/include/soci/firebird/soci-firebird.h index 361fdb678..50a66439d 100644 --- a/include/soci/firebird/soci-firebird.h +++ b/include/soci/firebird/soci-firebird.h @@ -266,6 +266,8 @@ struct firebird_blob_backend : details::blob_backend std::size_t append(const void *buf, std::size_t toWrite) override; void trim(std::size_t newLen) override; + details::session_backend &get_session_backend() override; + // Writes the current data into the database by allocating a new BLOB // object for it. // diff --git a/include/soci/odbc/soci-odbc.h b/include/soci/odbc/soci-odbc.h index bca300725..960396cc3 100644 --- a/include/soci/odbc/soci-odbc.h +++ b/include/soci/odbc/soci-odbc.h @@ -306,6 +306,7 @@ struct odbc_blob_backend : details::blob_backend std::size_t write_from_start(const void *buf, std::size_t toWrite, std::size_t offset = 0) override; std::size_t append(const void *buf, std::size_t toWrite) override; void trim(std::size_t newLen) override; + details::session_backend &get_session_backend() override; odbc_session_backend &session_; }; diff --git a/include/soci/oracle/soci-oracle.h b/include/soci/oracle/soci-oracle.h index 0f7b9eb1d..de21f0979 100644 --- a/include/soci/oracle/soci-oracle.h +++ b/include/soci/oracle/soci-oracle.h @@ -285,6 +285,8 @@ struct oracle_blob_backend : details::blob_backend void ensure_initialized(); + details::session_backend &get_session_backend() override; + private: std::size_t do_deprecated_read(std::size_t offset, void *buf, std::size_t toRead) override { diff --git a/include/soci/postgresql/soci-postgresql.h b/include/soci/postgresql/soci-postgresql.h index f13858d18..20fee4220 100644 --- a/include/soci/postgresql/soci-postgresql.h +++ b/include/soci/postgresql/soci-postgresql.h @@ -370,6 +370,8 @@ class postgresql_blob_backend : public details::blob_backend void reset(); + details::session_backend &get_session_backend() override; + private: postgresql_session_backend & session_; blob_details details_; diff --git a/include/soci/soci-backend.h b/include/soci/soci-backend.h index f92e0d2ae..bed27aa41 100644 --- a/include/soci/soci-backend.h +++ b/include/soci/soci-backend.h @@ -294,6 +294,8 @@ class rowid_backend virtual ~rowid_backend() {} }; +class session_backend; + // polymorphic blob backend class blob_backend @@ -312,6 +314,8 @@ class blob_backend virtual void trim(std::size_t newLen) = 0; + virtual session_backend &get_session_backend() = 0; + // Deprecated functions with backend-specific semantics preserved only for // compatibility. [[deprecated("Use read_from_start instead")]] diff --git a/src/backends/db2/blob.cpp b/src/backends/db2/blob.cpp index 006b6ce47..fc274c630 100644 --- a/src/backends/db2/blob.cpp +++ b/src/backends/db2/blob.cpp @@ -53,6 +53,11 @@ void db2_blob_backend::trim(std::size_t /* newLen */) throw soci_error("BLOBs are not supported."); } +details::session_backend &db2_blob_backend::get_session_backend() +{ + return session_; +} + #ifdef _MSC_VER # pragma warning(pop) #endif diff --git a/src/backends/empty/blob.cpp b/src/backends/empty/blob.cpp index db26321f9..0ff4e4860 100644 --- a/src/backends/empty/blob.cpp +++ b/src/backends/empty/blob.cpp @@ -53,6 +53,11 @@ void empty_blob_backend::trim(std::size_t /* newLen */) throw soci_error("BLOBs are not supported."); } +details::session_backend &empty_blob_backend::get_session_backend() +{ + return session_; +} + #ifdef _MSC_VER # pragma warning(pop) #endif diff --git a/src/backends/firebird/blob.cpp b/src/backends/firebird/blob.cpp index c646428b6..b4e401b7b 100644 --- a/src/backends/firebird/blob.cpp +++ b/src/backends/firebird/blob.cpp @@ -325,3 +325,8 @@ long firebird_blob_backend::getBLOBInfo() return total_length; } + +details::session_backend &firebird_blob_backend::get_session_backend() +{ + return session_; +} diff --git a/src/backends/mysql/blob.cpp b/src/backends/mysql/blob.cpp index a6d673aa0..5496914b3 100644 --- a/src/backends/mysql/blob.cpp +++ b/src/backends/mysql/blob.cpp @@ -20,8 +20,8 @@ using namespace soci; using namespace soci::details; -mysql_blob_backend::mysql_blob_backend(mysql_session_backend &) - : details::trivial_blob_backend() +mysql_blob_backend::mysql_blob_backend(mysql_session_backend &backend) + : details::trivial_blob_backend(backend) { } diff --git a/src/backends/odbc/blob.cpp b/src/backends/odbc/blob.cpp index b623533f0..af13dbf3d 100644 --- a/src/backends/odbc/blob.cpp +++ b/src/backends/odbc/blob.cpp @@ -53,6 +53,11 @@ void odbc_blob_backend::trim(std::size_t /* newLen */) throw soci_error("BLOBs are not supported."); } +details::session_backend &odbc_blob_backend::get_session_backend() +{ + return session_; +} + #ifdef _MSC_VER # pragma warning(pop) #endif diff --git a/src/backends/oracle/blob.cpp b/src/backends/oracle/blob.cpp index dd43b996d..5659af443 100644 --- a/src/backends/oracle/blob.cpp +++ b/src/backends/oracle/blob.cpp @@ -239,3 +239,8 @@ void oracle_blob_backend::ensure_initialized() initialized_ = true; } } + +details::session_backend &oracle_blob_backend::get_session_backend() +{ + return session_; +} diff --git a/src/backends/postgresql/blob.cpp b/src/backends/postgresql/blob.cpp index 8443f1312..757bceb9e 100644 --- a/src/backends/postgresql/blob.cpp +++ b/src/backends/postgresql/blob.cpp @@ -321,3 +321,8 @@ void postgresql_blob_backend::clone() lo_close(session_.conn_, old_details.fd); } } + +details::session_backend &postgresql_blob_backend::get_session_backend() +{ + return session_; +} diff --git a/src/backends/sqlite3/blob.cpp b/src/backends/sqlite3/blob.cpp index 28e2bf803..14a0178da 100644 --- a/src/backends/sqlite3/blob.cpp +++ b/src/backends/sqlite3/blob.cpp @@ -13,8 +13,8 @@ using namespace soci; -sqlite3_blob_backend::sqlite3_blob_backend(sqlite3_session_backend &) - : details::trivial_blob_backend() +sqlite3_blob_backend::sqlite3_blob_backend(sqlite3_session_backend &backend) + : details::trivial_blob_backend(backend) { } diff --git a/src/core/blob.cpp b/src/core/blob.cpp index dfd3b9895..e3bd7ecfe 100644 --- a/src/core/blob.cpp +++ b/src/core/blob.cpp @@ -28,7 +28,12 @@ bool blob::is_valid() const void blob::initialize(session &session) { - backEnd_.reset(session.make_blob_backend()); + initialize(session.make_blob_backend()); +} + +void blob::initialize(details::blob_backend *backend) +{ + backEnd_.reset(backend); } std::size_t blob::get_len() diff --git a/src/core/row.cpp b/src/core/row.cpp index 9e381b032..fe59e1412 100644 --- a/src/core/row.cpp +++ b/src/core/row.cpp @@ -121,7 +121,7 @@ blob row::move_as(std::size_t pos) const type_conversion::move_from_base(baseVal, *indicators_.at(pos), ret); // Re-initialize blob object so it can be used in further queries - baseVal.initialize(session); + baseVal.initialize(ret.get_backend()->get_session_backend().make_blob_backend()); return ret; } From 6c35d7724120e70cace2cae713a1c73bd0e1a5d2 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 1 May 2024 16:06:15 +0200 Subject: [PATCH 3/4] Adapt test case --- tests/common-tests.h | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/common-tests.h b/tests/common-tests.h index 504e53ea8..803b826d7 100644 --- a/tests/common-tests.h +++ b/tests/common-tests.h @@ -6853,17 +6853,21 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]") SECTION("Blob binding") { // Add data - soci::blob blob(sql); + soci::blob blob1(sql); + soci::blob blob2(sql); static_assert(10 <= sizeof(dummy_data), "Underlying assumption violated"); - blob.write_from_start(dummy_data, 10); - const int id = 42; - sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id), soci::use(blob); + blob1.write_from_start(dummy_data, 10); + blob2.write_from_start(dummy_data, 10); + const int id1 = 42; + const int id2 = 42; + sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id1), soci::use(blob1); + sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id2), soci::use(blob2); SECTION("into") { soci::blob intoBlob(sql); - sql << "select b from soci_test where id=:id", soci::use(id), soci::into(intoBlob); + sql << "select b from soci_test where id=:id", soci::use(id1), soci::into(intoBlob); char buffer[20]; std::size_t written = intoBlob.read_from_start(buffer, sizeof(buffer)); @@ -6875,8 +6879,7 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]") } SECTION("move_as") { - //soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id)); - soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id union all select b from soci_test where id=:id", soci::use(id, "id")); + soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id1)); bool containedData = false; for (auto it = rowSet.begin(); it != rowSet.end(); ++it) { @@ -6898,8 +6901,8 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]") } SECTION("reusing bound blob") { - int secondID = id + 1; - sql << "insert into soci_test(id, b) values(:id, :b)", soci::use(secondID), soci::use(blob); + int secondID = id2 + 1; + sql << "insert into soci_test(id, b) values(:id, :b)", soci::use(secondID), soci::use(blob2); // Selecting the blob associated with secondID should yield the same result as selecting the one for id soci::blob intoBlob(sql); From f110c69494967fd990c6be93403852d840d98346 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Sun, 1 Sep 2024 09:46:33 +0200 Subject: [PATCH 4/4] Fix blob fetching semantics in Oracle Previously, the into-type would take the blob object's LOB locator object and bind that in the query. That works as long as there is only a single Blob object being fetched from the database. However, if multiple values are fetched consecutively, there would only be a single locator object involved. In the best case, this leads to every new fetch overriding the previous result and in the worst case, that previous result has been turned into an independent blob object that has gone out of scope, which leads to freeing of this shared locator. With this commit, the into-type object will create its own locator object and use that. When assigning the fetched result to a blob object, the locator is copied such that the into-type and the blob keep independent locator objects, i.e. the locator is no longer shared. --- include/soci/oracle/soci-oracle.h | 2 +- src/backends/oracle/blob.cpp | 8 ++++++-- src/backends/oracle/standard-into-type.cpp | 24 ++++++++++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/include/soci/oracle/soci-oracle.h b/include/soci/oracle/soci-oracle.h index de21f0979..4ac7e8449 100644 --- a/include/soci/oracle/soci-oracle.h +++ b/include/soci/oracle/soci-oracle.h @@ -279,7 +279,7 @@ struct oracle_blob_backend : details::blob_backend locator_t get_lob_locator() const; - void set_lob_locator(locator_t locator, bool initialized = true); + void set_lob_locator(const locator_t locator, bool initialized = true); void reset(); diff --git a/src/backends/oracle/blob.cpp b/src/backends/oracle/blob.cpp index 5659af443..ba77d87cb 100644 --- a/src/backends/oracle/blob.cpp +++ b/src/backends/oracle/blob.cpp @@ -146,7 +146,7 @@ oracle_blob_backend::locator_t oracle_blob_backend::get_lob_locator() const return lobp_; } -void oracle_blob_backend::set_lob_locator(oracle_blob_backend::locator_t locator, bool initialized) +void oracle_blob_backend::set_lob_locator(const oracle_blob_backend::locator_t locator, bool initialized) { // If we select a BLOB value into a BLOB object, then the post_fetch code in // the standard_into_type_backend will set this object's locator to the one it is @@ -157,7 +157,11 @@ void oracle_blob_backend::set_lob_locator(oracle_blob_backend::locator_t locator { reset(); - lobp_ = locator; + sword res = OCILobLocatorAssign(session_.svchp_, session_.errhp_, locator, &lobp_); + if (res != OCI_SUCCESS) + { + throw_oracle_soci_error(res, session_.errhp_); + } } initialized_ = initialized; diff --git a/src/backends/oracle/standard-into-type.cpp b/src/backends/oracle/standard-into-type.cpp index 87ba49111..89eb05936 100644 --- a/src/backends/oracle/standard-into-type.cpp +++ b/src/backends/oracle/standard-into-type.cpp @@ -23,6 +23,8 @@ #include #include +#include + #ifdef _MSC_VER #pragma warning(disable:4355) #endif @@ -157,11 +159,24 @@ void oracle_standard_into_type_backend::define_by_pos( oracle_blob_backend *bbe = static_cast(b->get_backend()); - // Reset the blob to ensure that a potentially open temporary BLOB gets - // freed before the locator is changed to point to a different BLOB by the - // to-be-executed statement (which would leave us with a dangling temporary BLOB) + // Reset the blob to ensure that even when the query fails, + // the blob does not contain leftover data from whatever it + // contained before. bbe->reset(); - ociData_ = bbe->get_lob_locator(); + + // Allocate our very own LOB locator. We have to do this instead of + // reusing the locator from the existing Blob object in case the + // statement for which we are binding is going to be executed multiple + // times. If we borrowed the locator from the Blob object, consecutive + // queries would override a previous locator (which might belong + // to an independent Blob object by now). + sword res = OCIDescriptorAlloc(statement_.session_.envhp_, + reinterpret_cast(&ociData_), OCI_DTYPE_LOB, 0, 0); + if (res != OCI_SUCCESS) + { + throw soci_error("Cannot allocate the LOB locator"); + } + size = sizeof(ociData_); data = &ociData_; @@ -384,6 +399,7 @@ void oracle_standard_into_type_backend::clean_up() if (type_ == x_blob) { + OCIDescriptorFree(ociData_, OCI_DTYPE_LOB); ociData_ = NULL; }