Skip to content

Commit a2d3fcc

Browse files
committed
Fix #1599: Remove SQLCancel from core_sqlsrv_next_result catch block
When SQLMoreResults returns SQL_ERROR for a mid-batch statement failure (e.g. divide-by-zero with XACT_ABORT OFF), the statement handle remains valid. The old code called SQLCancel() in the catch block, which aborted the entire remaining batch. This change: 1. Removes SQLCancel from the catch block so the handle stays alive. 2. Adds a throw_on_errors parameter (default true) to core_sqlsrv_next_result to scope the non-throwing behaviour: - throw_on_errors=true (flush loops, closeCursor, param binding): uses the throwing core::SQLMoreResults wrapper — SQL_ERROR exits the loop via exception. Same behaviour as before. - throw_on_errors=false (sqlsrv_next_result / PDO::nextRowset): calls SQLMoreResults directly, reports the error through the normal error handler, clears any pending PDO exception, and falls through to new_result_set(). The batch remains navigable. 3. Tests both extensions (ERRMODE_WARNING and ERRMODE_EXCEPTION for PDO) and verifies re-execute after a batch error (flush loop). Fixes #1599
1 parent 204c4de commit a2d3fcc

5 files changed

Lines changed: 256 additions & 4 deletions

File tree

source/pdo_sqlsrv/pdo_stmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1217,7 +1217,7 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt )
12171217

12181218
SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" );
12191219

1220-
core_sqlsrv_next_result( static_cast<sqlsrv_stmt*>( stmt->driver_data ) );
1220+
core_sqlsrv_next_result( static_cast<sqlsrv_stmt*>( stmt->driver_data ), true, false );
12211221

12221222
if( driver_stmt->past_next_result_end == true ) {
12231223
// Clean up remaining metadata since new_result_set() was not called

source/shared/core_stmt.cpp

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
//---------------------------------------------------------------------------------------------------------------------------------
1919

2020
#include "core_sqlsrv.h"
21+
#include "zend_exceptions.h"
2122

2223
#include <sstream>
2324
#include <vector>
@@ -982,11 +983,40 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt, _In_ bool finalize_outp
982983
zend_hash_clean( Z_ARRVAL( stmt->col_cache ));
983984

984985
SQLRETURN r;
986+
985987
if( throw_on_errors ) {
988+
// Use the throwing wrapper. This is the default path used by
989+
// internal flush loops (re-execute, closeCursor, param binding)
990+
// where SQL_ERROR should abort the loop via exception.
986991
r = core::SQLMoreResults( stmt );
987992
}
988993
else {
989-
r = SQLMoreResults( stmt->handle() );
994+
// User-facing path (sqlsrv_next_result / PDO::nextRowset).
995+
// The ODBC driver may return SQL_ERROR instead of
996+
// SQL_SUCCESS_WITH_INFO for a failed statement that is not the
997+
// last in a non-aborted batch (e.g. divide-by-zero with
998+
// XACT_ABORT OFF). In that case the statement handle is still
999+
// valid and subsequent result sets remain reachable. We report
1000+
// the error through the normal handler but do NOT throw, so the
1001+
// caller sees success and the user can keep advancing.
1002+
r = ::SQLMoreResults( stmt->handle() );
1003+
1004+
SQLSRV_ASSERT( r != SQL_INVALID_HANDLE, "Invalid handle returned from SQLMoreResults." );
1005+
1006+
if( r == SQL_ERROR ) {
1007+
// Report the ODBC error so it is visible via
1008+
// sqlsrv_errors() / PDOStatement::errorInfo().
1009+
call_error_handler( stmt, SQLSRV_ERROR_ODBC, /*warning*/0 );
1010+
// The PDO error handler may set a pending zend exception
1011+
// when the error mode is PDO::ERRMODE_EXCEPTION. Clear it
1012+
// so execution continues to new_result_set() and the batch
1013+
// remains navigable. The error is still available via
1014+
// errorInfo().
1015+
zend_clear_exception();
1016+
}
1017+
else if( r == SQL_SUCCESS_WITH_INFO ) {
1018+
call_error_handler( stmt, SQLSRV_ERROR_ODBC, /*warning*/1 );
1019+
}
9901020
}
9911021

9921022
if( r == SQL_NO_DATA ) {
@@ -1005,7 +1035,11 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt, _In_ bool finalize_outp
10051035
}
10061036
catch( core::CoreException& e ) {
10071037

1008-
SQLCancel( stmt->handle() );
1038+
// Do not call SQLCancel here. When SQLMoreResults returns SQL_ERROR
1039+
// for a mid-batch statement failure (e.g. divide-by-zero with
1040+
// XACT_ABORT OFF), the statement handle is still valid and the user
1041+
// should be able to call next_result again to reach subsequent result
1042+
// sets. Calling SQLCancel would abort the entire remaining batch.
10091043
throw e;
10101044
}
10111045
}

source/sqlsrv/stmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ PHP_FUNCTION( sqlsrv_next_result )
569569

570570
try {
571571

572-
core_sqlsrv_next_result( stmt, true );
572+
core_sqlsrv_next_result( stmt, true, false );
573573

574574
if( stmt->past_next_result_end ) {
575575
// Clean up remaining metadata since new_result_set() was not called
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
--TEST--
2+
PDO nextRowset continues after mid-batch error with XACT_ABORT OFF
3+
--DESCRIPTION--
4+
When a multi-statement batch contains a failing statement (e.g. divide-by-zero)
5+
with XACT_ABORT OFF, nextRowset() should report the error but still allow
6+
subsequent nextRowset() calls to reach the remaining result sets. Previously
7+
the driver called SQLCancel() which destroyed the remaining batch.
8+
9+
This test verifies both ERRMODE_WARNING and ERRMODE_EXCEPTION modes.
10+
--SKIPIF--
11+
<?php require('skipif_mid-refactor.inc'); ?>
12+
--FILE--
13+
<?php
14+
require_once("MsCommon_mid-refactor.inc");
15+
16+
$batch = "SET XACT_ABORT OFF; SELECT 1 AS n; SELECT 1/0 AS boom; SELECT 2 AS n;";
17+
18+
// ============================================================
19+
// Test 1: ERRMODE_WARNING — error reported, batch continues
20+
// ============================================================
21+
echo "=== Test 1: ERRMODE_WARNING ===\n";
22+
23+
try {
24+
$conn = connect("", array(), PDO::ERRMODE_WARNING);
25+
$stmt = $conn->query($batch);
26+
27+
// Result set 1
28+
$row = $stmt->fetch(PDO::FETCH_ASSOC);
29+
echo "Result set 1: n={$row['n']}\n";
30+
31+
// Advance past failing SELECT 1/0 — returns true (batch not aborted)
32+
$next = $stmt->nextRowset();
33+
echo "nextRowset (failing): ";
34+
var_dump($next);
35+
36+
$err = $stmt->errorInfo();
37+
echo "Error SQLSTATE: {$err[0]}\n";
38+
39+
// Advance to result set 3
40+
$next2 = $stmt->nextRowset();
41+
echo "nextRowset (SELECT 2): ";
42+
var_dump($next2);
43+
44+
$row2 = $stmt->fetch(PDO::FETCH_ASSOC);
45+
echo "Result set 3: n={$row2['n']}\n";
46+
47+
$stmt = null;
48+
$conn = null;
49+
} catch (PDOException $e) {
50+
echo "UNEXPECTED Exception: " . $e->getMessage() . "\n";
51+
}
52+
53+
// ============================================================
54+
// Test 2: ERRMODE_EXCEPTION — exception cleared internally,
55+
// batch remains navigable, error in errorInfo()
56+
// ============================================================
57+
echo "\n=== Test 2: ERRMODE_EXCEPTION ===\n";
58+
59+
try {
60+
$conn = connect("", array(), PDO::ERRMODE_EXCEPTION);
61+
$stmt = $conn->query($batch);
62+
63+
// Result set 1
64+
$row = $stmt->fetch(PDO::FETCH_ASSOC);
65+
echo "Result set 1: n={$row['n']}\n";
66+
67+
// Advance past failing SELECT 1/0 — the driver clears the pending
68+
// exception internally so that the batch remains navigable.
69+
$next = $stmt->nextRowset();
70+
echo "nextRowset (failing): ";
71+
var_dump($next);
72+
73+
// Error info is still populated even though the exception was cleared.
74+
$err = $stmt->errorInfo();
75+
echo "Error SQLSTATE: {$err[0]}\n";
76+
77+
// Advance to result set 3
78+
$next2 = $stmt->nextRowset();
79+
echo "nextRowset (SELECT 2): ";
80+
var_dump($next2);
81+
82+
$row2 = $stmt->fetch(PDO::FETCH_ASSOC);
83+
echo "Result set 3: n={$row2['n']}\n";
84+
85+
$stmt = null;
86+
$conn = null;
87+
} catch (PDOException $e) {
88+
echo "UNEXPECTED Exception: " . $e->getMessage() . "\n";
89+
}
90+
91+
echo "\nDone\n";
92+
?>
93+
--EXPECTF--
94+
=== Test 1: ERRMODE_WARNING ===
95+
Result set 1: n=1
96+
nextRowset (failing): bool(true)
97+
Error SQLSTATE: 22012
98+
nextRowset (SELECT 2): bool(true)
99+
Result set 3: n=2
100+
101+
=== Test 2: ERRMODE_EXCEPTION ===
102+
Result set 1: n=1
103+
nextRowset (failing): bool(true)
104+
Error SQLSTATE: 22012
105+
nextRowset (SELECT 2): bool(true)
106+
Result set 3: n=2
107+
108+
Done
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
--TEST--
2+
sqlsrv_next_result continues after mid-batch error with XACT_ABORT OFF
3+
--DESCRIPTION--
4+
When a multi-statement batch contains a failing statement (e.g. divide-by-zero)
5+
with XACT_ABORT OFF, sqlsrv_next_result() should report the error but still
6+
allow the user to advance to subsequent result sets. Previously the driver
7+
called SQLCancel() which destroyed the remaining batch.
8+
9+
This test also verifies:
10+
- Error information is available via sqlsrv_errors()
11+
- Re-executing the statement (flush loop) works after batch errors
12+
--SKIPIF--
13+
<?php require('skipif.inc'); ?>
14+
--FILE--
15+
<?php
16+
require_once("MsSetup.inc");
17+
require_once("MsCommon.inc");
18+
19+
$conn = connect();
20+
if ($conn === false) {
21+
fatalError("Could not connect.\n");
22+
}
23+
24+
$batch = "SET XACT_ABORT OFF; SELECT 1 AS n; SELECT 1/0 AS boom; SELECT 2 AS n;";
25+
26+
// ============================================================
27+
// Test 1: XACT_ABORT OFF — non-fatal error, batch continues
28+
// ============================================================
29+
echo "=== Test 1: XACT_ABORT OFF ===\n";
30+
31+
$stmt = sqlsrv_query($conn, $batch);
32+
if ($stmt === false) {
33+
fatalError("sqlsrv_query failed.\n");
34+
}
35+
36+
// Result set 1: SELECT 1
37+
$row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC);
38+
echo "Result set 1: n={$row['n']}\n";
39+
40+
// Advance past failing SELECT 1/0 — returns true (batch not aborted)
41+
$next = sqlsrv_next_result($stmt);
42+
echo "next_result (failing): ";
43+
var_dump($next);
44+
45+
// Error is available via sqlsrv_errors()
46+
$errors = sqlsrv_errors(SQLSRV_ERR_ERRORS);
47+
if ($errors) {
48+
echo "Error SQLSTATE: {$errors[0]['SQLSTATE']}\n";
49+
}
50+
51+
// Advance to result set 3: SELECT 2
52+
$next2 = sqlsrv_next_result($stmt);
53+
echo "next_result (SELECT 2): ";
54+
var_dump($next2);
55+
56+
$row2 = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC);
57+
echo "Result set 3: n={$row2['n']}\n";
58+
59+
// End of results
60+
$next3 = sqlsrv_next_result($stmt);
61+
echo "next_result (end): ";
62+
var_dump($next3);
63+
64+
sqlsrv_free_stmt($stmt);
65+
66+
// ============================================================
67+
// Test 2: Re-execute after mid-batch error (flush loop)
68+
// ============================================================
69+
echo "\n=== Test 2: Re-execute after error ===\n";
70+
71+
$stmt2 = sqlsrv_prepare($conn, $batch);
72+
if ($stmt2 === false) {
73+
fatalError("sqlsrv_prepare failed.\n");
74+
}
75+
76+
// First execution — consume only the first result set
77+
sqlsrv_execute($stmt2);
78+
$row = sqlsrv_fetch_array($stmt2, SQLSRV_FETCH_ASSOC);
79+
echo "First execute, result set 1: n={$row['n']}\n";
80+
81+
// Re-execute — the internal flush loop must handle the remaining
82+
// results (including the SQL_ERROR from SELECT 1/0) without failing.
83+
$ok = sqlsrv_execute($stmt2);
84+
echo "Re-execute: ";
85+
var_dump($ok);
86+
87+
// Verify the new execution produces correct results
88+
$row = sqlsrv_fetch_array($stmt2, SQLSRV_FETCH_ASSOC);
89+
echo "Second execute, result set 1: n={$row['n']}\n";
90+
91+
sqlsrv_free_stmt($stmt2);
92+
sqlsrv_close($conn);
93+
94+
echo "\nDone\n";
95+
?>
96+
--EXPECT--
97+
=== Test 1: XACT_ABORT OFF ===
98+
Result set 1: n=1
99+
next_result (failing): bool(true)
100+
Error SQLSTATE: 22012
101+
next_result (SELECT 2): bool(true)
102+
Result set 3: n=2
103+
next_result (end): NULL
104+
105+
=== Test 2: Re-execute after error ===
106+
First execute, result set 1: n=1
107+
Re-execute: bool(true)
108+
Second execute, result set 1: n=1
109+
110+
Done

0 commit comments

Comments
 (0)