FIX: Prepared statement silently fails on insert - SET NOCOUNT ON fixes it (#1590)

jahnvi480 · web-flow · commit 3c514aa6ec30 · 2026-04-01T00:05:25.000+05:30
* FIX: Prepared statement silently fails on insert - SET NOCOUNT ON fixes it

* Increasing code coverage report

* Fuxung failing test

* Removing catch block

* Address review: consolidate drain tests, use GetTempTableName, move cleanup outside try

- Merged pdo_stmt_dtor_drain_{results,multi_resultset,statistics}.phpt into
  pdo_insert_with_extra_result_sets.phpt (now has 6 test cases)
- Merged sqlsrv_stmt_dtor_drain_{results,multi_resultset,statistics}.phpt into
  sqlsrv_insert_with_extra_result_sets.phpt (now has 5 test cases)
- Use GetTempTableName/getTempTableName for unique table names
- Move cleanup outside try block with IF EXISTS guards
- Delete 6 redundant test files
diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp
@@ -502,6 +502,21 @@ int pdo_sqlsrv_stmt_dtor( _Inout_ pdo_stmt_t *stmt )
         driver_stmt->placeholders = NULL;
     }
 
+    // Consume all pending result sets before freeing the statement handle.
+    // When MARS is enabled, freeing a statement with unconsumed results causes
+    // the ODBC driver to cancel the batch execution, which can roll back
+    // uncommitted implicit transactions, leading to silent data loss.
+    // This can occur when triggers, SET STATISTICS, or SET NOCOUNT OFF
+    // generate additional result sets beyond the primary result.
+    if (driver_stmt->executed && !driver_stmt->past_next_result_end) {
+        close_active_stream(driver_stmt);
+        SQLRETURN r = SQL_SUCCESS;
+        while (r == SQL_SUCCESS || r == SQL_SUCCESS_WITH_INFO) {
+            r = SQLMoreResults(driver_stmt->handle());
+        }
+        driver_stmt->past_next_result_end = true;
+    }
+
     (( sqlsrv_stmt* )driver_stmt )->~sqlsrv_stmt();
 
     sqlsrv_free( driver_stmt );
diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp
@@ -1344,6 +1344,19 @@ void __cdecl sqlsrv_stmt_dtor( _Inout_ zend_resource *rsrc )
         }
     }
 
+    // Consume all pending result sets before freeing the statement handle.
+    // When MARS is enabled, freeing a statement with unconsumed results causes
+    // the ODBC driver to cancel the batch execution, which can roll back
+    // uncommitted implicit transactions, leading to silent data loss.
+    if (stmt->executed && !stmt->past_next_result_end) {
+        close_active_stream(stmt);
+        SQLRETURN r = SQL_SUCCESS;
+        while (r == SQL_SUCCESS || r == SQL_SUCCESS_WITH_INFO) {
+            r = SQLMoreResults(stmt->handle());
+        }
+        stmt->past_next_result_end = true;
+    }
+
     stmt->~ss_sqlsrv_stmt();
     sqlsrv_free( stmt );
     rsrc->ptr = NULL;
diff --git a/test/functional/pdo_sqlsrv/pdo_insert_with_extra_result_sets.phpt b/test/functional/pdo_sqlsrv/pdo_insert_with_extra_result_sets.phpt
@@ -0,0 +1,97 @@
+--TEST--
+Verify stmt dtor drains unconsumed result sets to prevent silent data loss
+--DESCRIPTION--
+When triggers, SET STATISTICS PROFILE ON, or multi-result batches produce extra
+result sets, the statement destructor must drain them before freeing the ODBC
+handle. Otherwise, with MARS enabled, freeing the handle cancels the batch and
+silently rolls back uncommitted implicit transactions.
+--SKIPIF--
+<?php require('skipif.inc'); ?>
+--FILE--
+<?php
+require_once('MsCommon.inc');
+
+$tableName = GetTempTableName('pdo_drain_results', false);
+$triggerName = 'trg_' . $tableName;
+
+try {
+    $conn = connect();
+
+    // Create test table
+    $conn->exec("IF OBJECT_ID('$tableName', 'U') IS NOT NULL DROP TABLE $tableName");
+    $conn->exec("CREATE TABLE $tableName (id INT IDENTITY(1,1) PRIMARY KEY, val VARCHAR(50))");
+
+    // Test 1: Prepared statement with SET STATISTICS PROFILE ON
+    $stmt = $conn->prepare("SET STATISTICS PROFILE ON; INSERT INTO $tableName (val) VALUES ('row1'), ('row2'), ('row3')");
+    $stmt->execute();
+    $stmt = null;
+
+    $count = $conn->query("SELECT COUNT(*) FROM $tableName")->fetchColumn();
+    echo "Test 1 (STATISTICS PROFILE): " . ($count == 3 ? "PASS" : "FAIL (got $count)") . "\n";
+
+    // Test 2: Prepared statement with STATISTICS PROFILE + NOCOUNT OFF
+    $conn->exec("DELETE FROM $tableName");
+    $stmt = $conn->prepare("SET STATISTICS PROFILE ON; SET NOCOUNT OFF; INSERT INTO $tableName (val) VALUES ('a'), ('b'), ('c'), ('d'), ('e')");
+    $stmt->execute();
+    $stmt = null;
+
+    $count = $conn->query("SELECT COUNT(*) FROM $tableName")->fetchColumn();
+    echo "Test 2 (STATISTICS + NOCOUNT): " . ($count == 5 ? "PASS" : "FAIL (got $count)") . "\n";
+
+    // Test 3: PDO::exec baseline (already worked before the fix)
+    $conn->exec("DELETE FROM $tableName");
+    $conn->exec("SET STATISTICS PROFILE ON; SET NOCOUNT OFF; INSERT INTO $tableName (val) VALUES ('x'), ('y')");
+    $conn2 = connect();
+    $count = $conn2->query("SELECT COUNT(*) FROM $tableName")->fetchColumn();
+    $conn2 = null;
+    echo "Test 3 (exec baseline): " . ($count == 2 ? "PASS" : "FAIL (got $count)") . "\n";
+
+    // Test 4: Trigger producing extra result sets
+    $conn->exec("SET STATISTICS PROFILE OFF");
+    $conn->exec("DELETE FROM $tableName");
+    $conn->exec("IF OBJECT_ID('$triggerName', 'TR') IS NOT NULL DROP TRIGGER $triggerName");
+    $conn->exec("
+        CREATE TRIGGER $triggerName ON $tableName
+        AFTER INSERT AS
+        BEGIN
+            SELECT COUNT(*) AS trigger_count FROM $tableName
+        END
+    ");
+
+    $stmt = $conn->prepare("INSERT INTO $tableName (val) VALUES ('t1'), ('t2'), ('t3')");
+    $stmt->execute();
+    unset($stmt);
+
+    $count = $conn->query("SELECT COUNT(*) FROM $tableName")->fetchColumn();
+    echo "Test 4 (trigger): " . ($count == 3 ? "PASS" : "FAIL (got $count)") . "\n";
+
+    // Test 5: Multi-result SELECT batch with partial consumption
+    $stmt = $conn->prepare("SELECT 1 AS a; SELECT 2 AS b; SELECT 3 AS c");
+    $stmt->execute();
+    $row = $stmt->fetch(PDO::FETCH_ASSOC);
+    echo "Test 5 (multi-result first): " . ($row['a'] == 1 ? "PASS" : "FAIL") . "\n";
+    unset($stmt);
+
+    $row = $conn->query("SELECT 42 AS answer")->fetch(PDO::FETCH_ASSOC);
+    echo "Test 5 (after drain): " . ($row['answer'] == 42 ? "PASS" : "FAIL") . "\n";
+
+} catch (PDOException $e) {
+    echo "Error: " . $e->getMessage() . "\n";
+}
+
+// Cleanup outside try block — runs even on failure
+$conn = connect();
+$conn->exec("IF OBJECT_ID('$triggerName', 'TR') IS NOT NULL DROP TRIGGER $triggerName");
+$conn->exec("IF OBJECT_ID('$tableName', 'U') IS NOT NULL DROP TABLE $tableName");
+$conn = null;
+
+echo "Done.\n";
+?>
+--EXPECT--
+Test 1 (STATISTICS PROFILE): PASS
+Test 2 (STATISTICS + NOCOUNT): PASS
+Test 3 (exec baseline): PASS
+Test 4 (trigger): PASS
+Test 5 (multi-result first): PASS
+Test 5 (after drain): PASS
+Done.
diff --git a/test/functional/sqlsrv/sqlsrv_insert_with_extra_result_sets.phpt b/test/functional/sqlsrv/sqlsrv_insert_with_extra_result_sets.phpt
@@ -0,0 +1,104 @@
+--TEST--
+Verify stmt dtor drains unconsumed result sets to prevent silent data loss (sqlsrv)
+--DESCRIPTION--
+When triggers, SET STATISTICS PROFILE ON, or multi-result batches produce extra
+result sets, the statement destructor must drain them before freeing the ODBC
+handle. Otherwise, with MARS enabled, freeing the handle cancels the batch and
+silently rolls back uncommitted implicit transactions.
+--SKIPIF--
+<?php require('skipif.inc'); ?>
+--FILE--
+<?php
+require_once('MsCommon.inc');
+
+$tableName = getTempTableName('sqlsrv_drain_results', false);
+$triggerName = 'trg_' . $tableName;
+
+$conn = connect();
+if ($conn === false) {
+    die(print_r(sqlsrv_errors(), true));
+}
+
+// Create test table
+dropTable($conn, $tableName);
+$stmt = sqlsrv_query($conn, "CREATE TABLE $tableName (id INT IDENTITY(1,1) PRIMARY KEY, val VARCHAR(50))");
+if ($stmt === false) die(print_r(sqlsrv_errors(), true));
+sqlsrv_free_stmt($stmt);
+
+// Test 1: sqlsrv_prepare + execute with STATISTICS PROFILE
+$stmt = sqlsrv_prepare($conn, "SET STATISTICS PROFILE ON; INSERT INTO $tableName (val) VALUES ('row1'), ('row2'), ('row3')");
+if ($stmt === false) die(print_r(sqlsrv_errors(), true));
+if (!sqlsrv_execute($stmt)) die(print_r(sqlsrv_errors(), true));
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_query($conn, "SELECT COUNT(*) AS cnt FROM $tableName");
+$row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC);
+echo "Test 1 (STATISTICS PROFILE): " . ($row['cnt'] == 3 ? "PASS" : "FAIL (got {$row['cnt']})") . "\n";
+sqlsrv_free_stmt($stmt);
+
+// Test 2: sqlsrv_query with STATISTICS PROFILE + NOCOUNT OFF
+$stmt = sqlsrv_query($conn, "DELETE FROM $tableName");
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_query($conn, "SET STATISTICS PROFILE ON; SET NOCOUNT OFF; INSERT INTO $tableName (val) VALUES ('a'), ('b'), ('c'), ('d'), ('e')");
+if ($stmt === false) die(print_r(sqlsrv_errors(), true));
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_query($conn, "SELECT COUNT(*) AS cnt FROM $tableName");
+$row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC);
+echo "Test 2 (STATISTICS + NOCOUNT): " . ($row['cnt'] == 5 ? "PASS" : "FAIL (got {$row['cnt']})") . "\n";
+sqlsrv_free_stmt($stmt);
+
+// Test 3: Trigger producing extra result sets
+$stmt = sqlsrv_query($conn, "SET STATISTICS PROFILE OFF");
+sqlsrv_free_stmt($stmt);
+$stmt = sqlsrv_query($conn, "DELETE FROM $tableName");
+sqlsrv_free_stmt($stmt);
+
+sqlsrv_query($conn, "IF OBJECT_ID('$triggerName', 'TR') IS NOT NULL DROP TRIGGER $triggerName");
+$stmt = sqlsrv_query($conn, "
+    CREATE TRIGGER $triggerName ON $tableName
+    AFTER INSERT AS
+    BEGIN
+        SELECT COUNT(*) AS trigger_count FROM $tableName
+    END
+");
+if ($stmt === false) die(print_r(sqlsrv_errors(), true));
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_prepare($conn, "INSERT INTO $tableName (val) VALUES ('t1'), ('t2'), ('t3')");
+if (!sqlsrv_execute($stmt)) die(print_r(sqlsrv_errors(), true));
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_query($conn, "SELECT COUNT(*) AS cnt FROM $tableName");
+$row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC);
+echo "Test 3 (trigger): " . ($row['cnt'] == 3 ? "PASS" : "FAIL (got {$row['cnt']})") . "\n";
+sqlsrv_free_stmt($stmt);
+
+// Test 4: Multi-result SELECT batch with partial consumption
+$stmt = sqlsrv_query($conn, "SELECT 1 AS a; SELECT 2 AS b; SELECT 3 AS c");
+sqlsrv_fetch($stmt);
+$first = sqlsrv_get_field($stmt, 0);
+echo "Test 4 (multi-result first): " . ($first == 1 ? "PASS" : "FAIL") . "\n";
+sqlsrv_free_stmt($stmt);
+
+$stmt = sqlsrv_query($conn, "SELECT 42 AS answer");
+sqlsrv_fetch($stmt);
+$answer = sqlsrv_get_field($stmt, 0);
+echo "Test 4 (after drain): " . ($answer == 42 ? "PASS" : "FAIL") . "\n";
+sqlsrv_free_stmt($stmt);
+
+// Cleanup — runs regardless of test outcome
+sqlsrv_query($conn, "IF OBJECT_ID('$triggerName', 'TR') IS NOT NULL DROP TRIGGER $triggerName");
+dropTable($conn, $tableName);
+sqlsrv_close($conn);
+
+echo "Done.\n";
+?>
+--EXPECT--
+Test 1 (STATISTICS PROFILE): PASS
+Test 2 (STATISTICS + NOCOUNT): PASS
+Test 3 (trigger): PASS
+Test 4 (multi-result first): PASS
+Test 4 (after drain): PASS
+Done.
