FIx: Do not call SQLCancel in core_sqlsrv_next_result after SQLMoreResults error#1600
Open
FIx: Do not call SQLCancel in core_sqlsrv_next_result after SQLMoreResults error#1600
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1600 +/- ##
=======================================
Coverage 85.74% 85.75%
=======================================
Files 23 23
Lines 7221 7230 +9
=======================================
+ Hits 6192 6200 +8
- Misses 1029 1030 +1
🚀 New features to boost your workflow:
|
e7c13ef to
0b73262
Compare
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
0b73262 to
a2d3fcc
Compare
…ing APIs When SQLMoreResults returns SQL_ERROR for a mid-batch statement failure (e.g. divide-by-zero with XACT_ABORT OFF), the ODBC spec says it should return SQL_SUCCESS_WITH_INFO when the batch is not aborted and the failed statement is not the last one. The Microsoft ODBC driver incorrectly returns SQL_ERROR, but the statement handle remains valid. The old code called SQLCancel() in the catch block, which aborted the entire remaining batch, making subsequent result sets unreachable. This change adds a throw_on_errors parameter (default true) to core_sqlsrv_next_result: - throw_on_errors=true (default): used by internal flush loops (closeCursor, re-execute, param binding). Uses the throwing core::SQLMoreResults wrapper and calls SQLCancel in the catch block. Same behavior as before. - throw_on_errors=false: used only by sqlsrv_next_result() and 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. SQLCancel is NOT called in the catch block for this path. Tests cover both extensions (ERRMODE_WARNING and ERRMODE_EXCEPTION for PDO), re-execute after a batch error (flush loop), and error visibility via sqlsrv_errors() / errorInfo(). Fixes #1599
647cc77 to
6f911c2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Github issue #1599
This pull request improves how the SQLSRV and PDO_SQLSRV drivers handle errors during multi-statement batches, specifically when a non-fatal error (such as divide-by-zero) occurs mid-batch with
XACT_ABORT OFF. Previously, the driver would callSQLCancel()on any error, which destroyed the remaining result sets. Now, errors are reported but the batch remains navigable, allowing users to continue to subsequent result sets. The changes also ensure that error information is available without interrupting batch navigation, and add comprehensive tests for these scenarios.Error handling and batch navigation improvements:
core_sqlsrv_next_resultto distinguish between internal and user-facing error handling: for user-facing calls (likesqlsrv_next_resultandPDO::nextRowset), SQL errors no longer triggerSQLCancel(), allowing continued navigation through remaining result sets after a mid-batch error. Errors are reported via standard error handlers, and any pending Zend exceptions are cleared to avoid aborting navigation.pdo_stmt.cpp,stmt.cpp) to support the new error handling logic by passing additional parameters tocore_sqlsrv_next_result.Test coverage:
pdo_batch_error_continue.phptto test thatPDO::nextRowset()reports errors but allows continued navigation after a mid-batch error, for bothERRMODE_WARNINGandERRMODE_EXCEPTIONmodes.sqlsrv_batch_error_continue.phptto verify thatsqlsrv_next_result()behaves correctly after a mid-batch error, that error info is available, and that re-execution works after such errors.Dependency and include updates:
zend_exceptions.hincore_stmt.cppto enable Zend exception management for the improved error handling logic.