From 37535c2a8c7cb336dd4a81daf961c68bbc6b2151 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vz-soci@zeitlins.org>
Date: Tue, 28 Jan 2025 16:03:59 +0100
Subject: [PATCH] Tighten the checks in get_affected_rows() unit tests

Check that a partial update does throw an exception instead of checking
that it may throw it and check that get_affected_rows() returns the
actual number of affected rows instead of returning something non zero.

The only remaining exclusion is the PostgreSQL ODBC driver which just
seems to be buggy, notably in its versions < 13.02 when it inserted
valid rows into the database but still returned fatal error (with later
versions it doesn't insert anything, which is inconsistent with the
other drivers and unhelpful, but not quite as bad).
---
 tests/common/test-common.cpp        | 24 +++++++++---------------
 tests/odbc/test-odbc-postgresql.cpp | 15 +++++++++++++++
 tests/test-context.h                |  4 ++++
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/tests/common/test-common.cpp b/tests/common/test-common.cpp
index 6b9ee252d..95a895e03 100644
--- a/tests/common/test-common.cpp
+++ b/tests/common/test-common.cpp
@@ -3256,29 +3256,23 @@ TEST_CASE_METHOD(common_tests, "Get affected rows", "[core][affected-rows]")
 
     CHECK(st5.get_affected_rows() == 5);
 
+    if (tc_.has_partial_update_bug())
+    {
+        WARN("Skipping partial update test due to a known backend bug");
+        return;
+    }
+
     std::vector<std::string> w(2, "1");
     w[1] = "a"; // this invalid value may cause an exception.
     statement st6 = (sql.prepare <<
         "insert into soci_test(val) values(:val)", use(w));
-    try { st6.execute(true); }
-    catch(...) {}
+    CHECK_THROWS_AS(st6.execute(true), soci_error);
+    CHECK(st6.get_affected_rows() == 1);
 
     // confirm the partial insertion.
     int val = 0;
     sql << "select count(val) from soci_test", into(val);
-    if(val != 0)
-    {
-        // Notice that some ODBC drivers don't return the number of updated
-        // rows at all in the case of partially executed statement like this
-        // one, while MySQL ODBC driver wrongly returns 2 affected rows even
-        // though only one was actually inserted.
-        //
-        // So we can't check for "get_affected_rows() == val" here, it would
-        // fail in too many cases -- just check that the backend doesn't lie to
-        // us about no rows being affected at all (even if it just honestly
-        // admits that it has no idea by returning -1).
-        CHECK(st6.get_affected_rows() != 0);
-    }
+    CHECK(val == 1);
 }
 
 // test fix for: Backend is not set properly with connection pool (pull #5)
diff --git a/tests/odbc/test-odbc-postgresql.cpp b/tests/odbc/test-odbc-postgresql.cpp
index f65062679..4e9366c00 100644
--- a/tests/odbc/test-odbc-postgresql.cpp
+++ b/tests/odbc/test-odbc-postgresql.cpp
@@ -225,6 +225,21 @@ class test_context : public test_context_common
         return !m_verDriver.is_initialized() || m_verDriver < odbc_version(9, 3, 400);
     }
 
+    bool has_partial_update_bug() const override
+    {
+        // ODBC driver has version-dependent bugs related to handling array
+        // parameters: after v13.02, it fails to insert anything, see
+        // https://github.com/postgresql-interfaces/psqlodbc/issues/89, and
+        // with the previous versions (e.g. v10.03) it's even worse, as it does
+        // insert the row with valid values but still returns fatal error at
+        // ODBC level.
+        //
+        // So far there is no known version where it works correctly, but if
+        // the issue above is fixed, we should check for the version including
+        // this fix here.
+        return true;
+    }
+
     std::string fix_crlf_if_necessary(std::string const& s) const override
     {
         // Version 9.03.0300 (ancient, but still used on AppVeyor CI) is known
diff --git a/tests/test-context.h b/tests/test-context.h
index e7554b874..58fdea556 100644
--- a/tests/test-context.h
+++ b/tests/test-context.h
@@ -193,6 +193,10 @@ class test_context_base
     // *exactly* the same value.
     virtual bool has_fp_bug() const { return false; }
 
+    // Override this if the backend doesn't handle partial success for
+    // operations using array parameters correctly.
+    virtual bool has_partial_update_bug() const { return false; }
+
     // Override this if the backend wrongly returns CR LF when reading a string
     // with just LFs from the database to strip the unwanted CRs.
     virtual std::string fix_crlf_if_necessary(std::string const& s) const { return s; }