Skip to content

FIx: Do not call SQLCancel in core_sqlsrv_next_result after SQLMoreResults error#1600

Open
jahnvi480 wants to merge 3 commits intodevfrom
jahnvi/batch-error-sqlcancel
Open

FIx: Do not call SQLCancel in core_sqlsrv_next_result after SQLMoreResults error#1600
jahnvi480 wants to merge 3 commits intodevfrom
jahnvi/batch-error-sqlcancel

Conversation

@jahnvi480
Copy link
Copy Markdown
Contributor

@jahnvi480 jahnvi480 commented Apr 14, 2026

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 call SQLCancel() 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:

  • Updated core_sqlsrv_next_result to distinguish between internal and user-facing error handling: for user-facing calls (like sqlsrv_next_result and PDO::nextRowset), SQL errors no longer trigger SQLCancel(), 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.
  • Modified function signatures and call sites (pdo_stmt.cpp, stmt.cpp) to support the new error handling logic by passing additional parameters to core_sqlsrv_next_result.

Test coverage:

  • Added pdo_batch_error_continue.phpt to test that PDO::nextRowset() reports errors but allows continued navigation after a mid-batch error, for both ERRMODE_WARNING and ERRMODE_EXCEPTION modes.
  • Added sqlsrv_batch_error_continue.phpt to verify that sqlsrv_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:

  • Included zend_exceptions.h in core_stmt.cpp to enable Zend exception management for the improved error handling logic.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.75%. Comparing base (9e82915) to head (1bbfdc9).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
source/shared/core_stmt.cpp 88.88% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #1600   +/-   ##
=======================================
  Coverage   85.74%   85.75%           
=======================================
  Files          23       23           
  Lines        7221     7230    +9     
=======================================
+ Hits         6192     6200    +8     
- Misses       1029     1030    +1     
Files with missing lines Coverage Δ
source/pdo_sqlsrv/pdo_stmt.cpp 81.71% <100.00%> (ø)
source/sqlsrv/stmt.cpp 88.31% <100.00%> (ø)
source/shared/core_stmt.cpp 93.48% <88.88%> (-0.04%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jahnvi480 jahnvi480 force-pushed the jahnvi/batch-error-sqlcancel branch 2 times, most recently from e7c13ef to 0b73262 Compare April 14, 2026 07:28
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
@jahnvi480 jahnvi480 force-pushed the jahnvi/batch-error-sqlcancel branch from 0b73262 to a2d3fcc Compare April 15, 2026 07:52
…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
@jahnvi480 jahnvi480 force-pushed the jahnvi/batch-error-sqlcancel branch from 647cc77 to 6f911c2 Compare April 17, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant