Address review comments

include/soci/blob.h

@@ -44,7 +44,12 @@ public:
     [[deprecated("Use read_from_start instead")]]
     std::size_t read(std::size_t offset, char * buf, std::size_t toRead);
 
-    // offset starts from 0
+    // Extracts data from this blob into the given buffer.
+    // At most toRead bytes are extracted (and copied into buf).
+    // The amount of actually read bytes is returned.
+    //
+    // Note: Using an offset > 0 on a blob whose size is less than
+    // or equal to offset, will throw an exception.
     std::size_t read_from_start(char * buf, std::size_t toRead,
         std::size_t offset = 0);
 

include/soci/firebird/soci-firebird.h

@@ -265,11 +265,11 @@ struct firebird_blob_backend : details::blob_backend
     std::size_t append(char const *buf, std::size_t toWrite) override;
     void trim(std::size_t newLen) override;
 
-    /// Writes the current data into the database by allocating a new BLOB
-    /// object for it.
-    ///
-    /// @returns The ID of the newly created BLOB object
-    ISC_QUAD write_to_db();
+    // Writes the current data into the database by allocating a new BLOB
+    // object for it.
+    //
+    // Returns The ID of the newly created BLOB object
+    ISC_QUAD save_to_db();
     void assign(ISC_QUAD const & bid);
 
 private:
@@ -278,7 +278,7 @@ private:
     void load();
     void writeBuffer(std::size_t offset, char const * buf,
         std::size_t toWrite);
-    void closeBlob(bool keepData);
+    void closeBlob();
 
     firebird_session_backend &session_;
     ISC_QUAD blob_id_;

include/soci/oracle/soci-oracle.h

@@ -304,6 +304,8 @@ private:
 
     locator_t lobp_;
 
+    // If this is true, then the locator lobp_ points to something useful
+    // (instead of being the equivalent to a pointer with random value)
     bool initialized_;
 };
 

include/soci/soci-backend.h

@@ -341,10 +341,10 @@ public:
             throw soci_error("Can't read past-the-end of BLOB data.");
         }
 
-        toRead = (std::min)(toRead, buffer_.size() - offset);
-
         // make sure that we don't try to read
         // past the end of the data
+        toRead = (std::min)(toRead, buffer_.size() - offset);
+
         memcpy(buf, buffer_.data() + offset, toRead);
 
         return toRead;

include/soci/type-traits.h

@@ -5,8 +5,8 @@
 // http://www.boost.org/LICENSE_1_0.txt)
 //
 
-#ifndef SOCI_PRIVATE_SOCI_TYPE_TRAITS_H_INCLUDED
-#define SOCI_PRIVATE_SOCI_TYPE_TRAITS_H_INCLUDED
+#ifndef SOCI_SOCI_TYPE_TRAITS_H_INCLUDED
+#define SOCI_SOCI_TYPE_TRAITS_H_INCLUDED
 
 #include <type_traits>
 
@@ -23,6 +23,9 @@ using false_type = std::integral_constant<bool, false>;
 using true_type = std::integral_constant<bool, true>;
 
 // Implementation from https://blog.tartanllama.xyz/detection-idiom/
+// Note, this is a stub that we require until standard C++ gets support
+// for the detection idiom that is not experimental (and thus can be
+// assumed to be present).
 
 namespace detector_detail
 {
@@ -43,4 +46,4 @@ using is_detected = typename detector_detail::is_detected<Trait, void, Args...>:
 
 }
 
-#endif // SOCI_PRIVATE_SOCI_TYPE_TRAITS_H_INCLUDED
+#endif // SOCI_SOCI_TYPE_TRAITS_H_INCLUDED

src/backends/firebird/blob.cpp

@@ -21,7 +21,7 @@ firebird_blob_backend::firebird_blob_backend(firebird_session_backend &session)
 
 firebird_blob_backend::~firebird_blob_backend()
 {
-    closeBlob(false);
+    closeBlob();
 }
 
 std::size_t firebird_blob_backend::get_len()
@@ -152,17 +152,12 @@ void firebird_blob_backend::open()
     data_.resize(blob_size);
 }
 
-void firebird_blob_backend::closeBlob(bool keepData)
+void firebird_blob_backend::closeBlob()
 {
     from_db_ = false;
     loaded_ = false;
     max_seg_size_ = 0;
 
-    if (!keepData)
-    {
-        data_.resize(0);
-    }
-
     if (blob_handle_ != 0)
     {
         // close blob
@@ -231,7 +226,7 @@ void firebird_blob_backend::load()
     loaded_ = true;
 }
 
-ISC_QUAD firebird_blob_backend::write_to_db()
+ISC_QUAD firebird_blob_backend::save_to_db()
 {
     // close old blob if necessary
     ISC_STATUS stat[20];
@@ -280,14 +275,15 @@ ISC_QUAD firebird_blob_backend::write_to_db()
     // In any case, BLOBs in Firebird can't be updated anyway - one always has to create a new BLOB object
     // (with a new ID) and then use that to modify the existing one (replace the ID in the corresponding table).
     // Therefore, keeping the Blob open for subsequent modification is not needed.
-    closeBlob(true);
+    closeBlob();
 
     return blob_id_;
 }
 
 void firebird_blob_backend::assign(const ISC_QUAD &id)
 {
-    closeBlob(false);
+    closeBlob();
+    data_.clear();
 
     blob_id_ = id;
     from_db_ = true;

src/backends/firebird/standard-use-type.cpp

@@ -158,7 +158,7 @@ void firebird_standard_use_type_backend::exchangeData()
                     throw soci_error("Can't get Firebid BLOB BackEnd");
                 }
 
-                ISC_QUAD blob_id = blob->write_to_db();
+                ISC_QUAD blob_id = blob->save_to_db();
                 memcpy(buf_, &blob_id, sizeof(blob_id));
             }
             break;
@@ -185,7 +185,7 @@ void firebird_standard_use_type_backend::copy_to_blob(const std::string& in)
 
     blob_->append(in.c_str(), in.length());
 
-    ISC_QUAD blob_id = blob_->write_to_db();
+    ISC_QUAD blob_id = blob_->save_to_db();
     memcpy(buf_, &blob_id, sizeof(blob_id));
 }
 

src/backends/firebird/vector-use-type.cpp

@@ -195,7 +195,7 @@ void firebird_vector_use_type_backend::copy_to_blob(const std::string &in)
 
     blob_ = new firebird_blob_backend(statement_.session_);
     blob_->append(in.c_str(), in.length());
-    ISC_QUAD blob_id = blob_->write_to_db();
+    ISC_QUAD blob_id = blob_->save_to_db();
     memcpy(buf_, &blob_id, sizeof(blob_id));
 }
 

src/backends/mysql/standard-use-type.cpp

@@ -178,6 +178,9 @@ void mysql_standard_use_type_backend::pre_use(indicator const *ind)
                 }
                 else
                 {
+                    // Note: since the entire MySQL works by assembling the query in text form,
+                    // we can't make use of proper blob objects (that'd require a more sophisticated
+                    // way of using the MySQL API).
                     buf_ = new char[hex_size + 1];
                     bbe->write_hex_str(buf_, hex_size);
                     // Add NULL terminator

src/backends/oracle/blob.cpp

@@ -14,8 +14,6 @@
 #include <ctime>
 #include <cctype>
 
-#include <iostream>
-
 #ifdef _MSC_VER
 #pragma warning(disable:4355)
 #endif
@@ -97,11 +95,11 @@ std::size_t oracle_blob_backend::read_from_start(char *buf, std::size_t toRead,
 
 std::size_t oracle_blob_backend::write_from_start(char const *buf, std::size_t toWrite, std::size_t offset)
 {
-	if (offset > get_len())
-	{
+    if (offset > get_len())
+    {
         // If offset == length, the operation is to be understood as appending (and is therefore allowed)
         throw soci_error("Can't start writing far past-the-end of BLOB data.");
-	}
+    }
 
     ensure_initialized();
 

src/backends/postgresql/blob.cpp

@@ -253,7 +253,7 @@ void postgresql_blob_backend::clone()
     }
 
     blob_details old_details = details_;
-	details_ = {};
+    details_ = {};
     reset();
     init();
 
@@ -282,15 +282,15 @@ void postgresql_blob_backend::clone()
         offset += sizeof(buf);
     } while (read_bytes == sizeof(buf));
 
-	// Dispose old BLOB object
-	if (destroy_on_close_)
+    // Dispose old BLOB object
+    if (destroy_on_close_)
     {
-		// Remove the large object from the DB completely
-		lo_unlink(session_.conn_, old_details.fd);
-	}
+        // Remove the large object from the DB completely
+        lo_unlink(session_.conn_, old_details.fd);
+    }
     else
     {
-		// Merely close our handle to the large object
-		lo_close(session_.conn_, old_details.fd);
-	}
+        // Merely close our handle to the large object
+        lo_close(session_.conn_, old_details.fd);
+    }
 }

src/backends/sqlite3/statement.cpp

@@ -327,10 +327,11 @@ sqlite3_statement_backend::bind_and_execute(int number)
                         break;
 
                     case db_blob:
-						// Since we don't own the buffer_ pointer we are passing here, we can't make any lifetime guarantees other than
-						// it is currently valid. Thus, we ask SQLite to make a copy of the underlying buffer to ensure
-						// the database can always access a valid buffer.
-                        bindRes = sqlite3_bind_blob(stmt_, pos, col.buffer_.constData_, static_cast<int>(col.buffer_.size_), SQLITE_TRANSIENT);
+                        // Since we don't own the buffer_ pointer we are passing here, we can't make any lifetime
+                        // guarantees other than it is currently valid. Thus, we ask SQLite to make a copy of the
+                        // underlying buffer to ensure the database can always access a valid buffer.
+                        bindRes = sqlite3_bind_blob(stmt_, pos, col.buffer_.constData_,
+                                static_cast<int>(col.buffer_.size_), SQLITE_TRANSIENT);
                         break;
 
                     case db_xml:

tests/common-tests.h

@@ -363,18 +363,6 @@ private:
     SOCI_NOT_COPYABLE(function_creator_base)
 };
 
-enum class Backend
-{
-    Empty,
-    SQLite,
-    MySQL,
-    PostgreSQL,
-    ODBC,
-    Oracle,
-    Firebird,
-    DB2,
-};
-
 // This is a singleton class, at any given time there is at most one test
 // context alive and common_tests fixture class uses it.
 class test_context_base
@@ -416,8 +404,6 @@ public:
         return connectString_;
     }
 
-    virtual Backend get_backend() const = 0;
-
     virtual std::string to_date_time(std::string const &dateTime) const = 0;
 
     virtual table_creator_base* table_creator_1(session&) const = 0;
@@ -6404,7 +6390,7 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
         return;
     }
 
-    const char dummy_data[] = "abcdefghijklmnopüqrstuvwxyz";
+    const char dummy_data[] = "abcdefghijklmnopqrstuvwxyz";
 
     // Cross-DB usage of BLOBs is only possible if the entire lifetime of the blob object
     // is covered in an active transaction.
@@ -6636,10 +6622,6 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
                 containedData = true;
                 const soci::row &currentRow = *it;
 
-                soci::data_type type = currentRow.get_properties(0).get_data_type();
-                soci::data_type expectedType = tc_.get_backend() != Backend::Oracle ? soci::dt_integer : soci::dt_long_long;
-                CHECK(type == expectedType);
-
                 CHECK(currentRow.get_properties(1).get_data_type() == soci::dt_blob);
             }
             CHECK(containedData);
@@ -6750,7 +6732,6 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
             CHECK(!select_stmt.fetch());
         }
     }
-    transaction.rollback();
 }
 
 TEST_CASE_METHOD(common_tests, "Logger", "[core][log]")

tests/db2/test-db2.cpp

@@ -68,8 +68,6 @@ public:
     test_context(backend_factory const & pi_back_end, std::string const & pi_connect_string)
         : test_context_base(pi_back_end, pi_connect_string) {}
 
-    Backend get_backend() const override { return  Backend::DB2; }
-
     table_creator_base* table_creator_1(soci::session & pr_s) const override
     {
         pr_s << "SET CURRENT SCHEMA = 'DB2INST1'";

tests/firebird/test-firebird.cpp

@@ -1191,8 +1191,6 @@ class test_context : public tests::test_context_base
                 : test_context_base(backEnd, connectString)
         {}
 
-        tests::Backend get_backend() const override { return tests::Backend::Firebird; }
-
         tests::table_creator_base* table_creator_1(soci::session& s) const override
         {
             return new TableCreator1(s);

tests/mysql/test-mysql.h

@@ -70,8 +70,6 @@ public:
                 std::string const &connectString)
         : test_context_base(backEnd, connectString) {}
 
-    Backend get_backend() const override { return Backend::MySQL; }
-
     table_creator_base* table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);
@@ -97,13 +95,10 @@ public:
         return "\'" + datdt_string + "\'";
     }
 
-#ifndef SOCI_INCLUDED_FROM_ODBC_TEST
-    // ODBC backend doesn't support BLOBs yet
-    table_creator_base* table_creator_blob(soci::session& s) const override
+    table_creator_base * table_creator_blob(soci::session &s) const override
     {
-      return new table_creator_for_blob(s);
+        return new table_creator_for_blob(s);
     }
-#endif
 
     bool has_fp_bug() const override
     {

tests/odbc/test-odbc-access.cpp

@@ -73,8 +73,6 @@ public:
     test_context(backend_factory const &backend, std::string const &connstr)
         : test_context_base(backend, connstr) {}
 
-    Backend get_backend() const override { return Backend::ODBC; }
-
     table_creator_base * table_creator_1(soci::session& s) const
     {
         return new table_creator_one(s);

tests/odbc/test-odbc-db2.cpp

@@ -69,8 +69,6 @@ public:
                 std::string const &connectString)
         : test_context_base(backEnd, connectString) {}
 
-    Backend get_backend() const override { return Backend::ODBC; }
-
     table_creator_base * table_creator_1(soci::session& s) const
     {
         return new table_creator_one(s);

tests/odbc/test-odbc-mssql.cpp

@@ -156,8 +156,6 @@ public:
                 std::string const &connstr)
         : test_context_base(backend, connstr) {}
 
-    Backend get_backend() const override { return Backend::ODBC; }
-
     table_creator_base* table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);

tests/odbc/test-odbc-mysql.cpp

@@ -12,9 +12,7 @@
 #include <ctime>
 #include <cmath>
 
-#define SOCI_INCLUDED_FROM_ODBC_TEST
 #include "mysql/test-mysql.h"
-#undef SOCI_INCLUDED_FROM_ODBC_TEST
 
 std::string connectString;
 backend_factory const &backEnd = *soci::factory_odbc();
@@ -34,6 +32,11 @@ public:
         // but we use an older version in the AppVeyor builds.
         return true;
     }
+
+    table_creator_base* table_creator_blob(soci::session& s) const override
+    {
+      return new table_creator_for_blob(s);
+    }
 };
 
 int main(int argc, char** argv)

tests/odbc/test-odbc-postgresql.cpp

@@ -161,8 +161,6 @@ public:
         std::cout << "Using ODBC driver version " << m_verDriver << "\n";
     }
 
-    Backend get_backend() const override { return Backend::ODBC; }
-
     table_creator_base * table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);

tests/oracle/test-oracle.cpp

@@ -1407,8 +1407,6 @@ public:
                 std::string const &connectString)
         : test_context_base(backEnd, connectString) {}
 
-    Backend get_backend() const override { return Backend::Oracle; }
-
     table_creator_base* table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);

tests/postgresql/test-postgresql.cpp

@@ -15,7 +15,6 @@
 #include <cstring>
 #include <ctime>
 #include <cstdlib>
-#include <pg_config.h>
 
 using namespace soci;
 using namespace soci::tests;
@@ -1346,8 +1345,6 @@ public:
         : test_context_base(backend, connstr)
     {}
 
-    Backend get_backend() const override { return Backend::PostgreSQL; }
-
     table_creator_base* table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);

tests/sqlite3/test-sqlite3.cpp

@@ -794,8 +794,6 @@ public:
                 std::string const &connstr)
         : test_context_base(backend, connstr) {}
 
-    Backend get_backend() const override { return Backend::SQLite; }
-
     table_creator_base* table_creator_1(soci::session& s) const override
     {
         return new table_creator_one(s);