You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #796, now merged into main.#796 reuses result-set metadata; this change reuses one statement-owned native buffer and ODBC binding plan across compatible, bounded-column fetchmany() calls.
Reuse requires the same result generation, metadata, batch size, and decoding settings. Checked cleanup protects driver-held pointers across incompatible transitions, errors, and close. Public APIs/defaults, Row/converter semantics, and MAX/LOB/sql_variant fallback routes remain unchanged; there is no hidden prefetch.
Repository scope (30893611 → 39e22098): five runtime files plus regressions in the existing tests/test_025_profiler.py. There is no effective build/CI or documentation diff. Three isolated cases use the existing profiler to assert actual bind/unbind counts for a compatible hit, a size/encoding/result-set miss, resumed reuse, and EOF. One additional focused test, parameterized for failed unbinding and failed rows-fetched-pointer cleanup, covers failure propagation, blocked reuse/rebinding/fetching, and recovery. These cases require a profiling-enabled native build and live SQL; unavailable profiling is explicitly skipped, not reported as zero calls. The failure-injection cases also skip Windows because its ODBC function-pointer globals are not exported there.
Scope correction:ad0a8550 reverted the four-file, 308-line cleanup-test increment 1c05d11e after the user rejected its size. The native test target, CMake/CI wiring, and related documentation remain removed. ad0a8550 restored the complete be219eb3 source tree; the follow-up through db19e637 added only one 93-line test in the existing test module. 39e22098 parameterizes that same test for the second cleanup-failure boundary (+26/-10 in that file only). There are no additional production, build, CI, or documentation changes. The earlier DevSkim fixes and three profiler regressions remain; history was preserved through a normal revert, not rewritten.
flowchart LR
subgraph "Before (#796)"
B["Each fetchmany"] --> S["Allocate + bind"] --> F["Fetch + unbind"]
end
subgraph After
A["First compatible call"] --> P["Owned buffers + bindings"]
P --> R["Reuse on subsequent fetchmany calls"]
R --> D["Detach at EOF / transition"]
end
Loading
Mechanism and correctness
Actual bind/unbind/attribute-call timers and plan-allocation profiling are preserved. The auxiliary value-buffer-growth timer was removed; its absence is not zero allocation work.
The source preserves the main merges through 30893611 and the two DevSkim fixes in 9eb586a6, which replace formatted C calls with native-only output. The reported cache/cleanup lock inversion was checked against the function-local mutex lifetimes: cache clearing returns and releases its lock before the cleanup gate is acquired; no teardown reordering was needed.
Prior SQL counter validation and source equivalence:Azure DevOps build 178157 checked out merge e3dc03c0, whose complete tree e29a5927 matches both be219eb3 and ad0a8550. Both Ubuntu x86_64 / Python 3.12.3 SQL Server 2022 and SQL Server 2025 legs completed fresh Release ON native builds and the existing checkout/profiling import guard. Each leg passed all three size/encoding/result regression cases, with zero selected skips. Each full pytest task reported 5,557 passed, 132 skipped, 42 deselected. These results remain attributed to that CI run and do not qualify the subsequently added cleanup-failure cases. Successful raw counters and native-binary hashes were not independently exported; this is not full-matrix qualification.
The original 9eb586a6 CI run exposed an unsupported UTF-16 alias in the encoding test before its transition fetch. be219eb3 corrected only two test-input lines to supported ASCII and Latin-1 settings with SQL_CHAR fixed; the original failure is retained in the evidence. This tests decoding-configuration invalidation, not a change in Linux's effective UTF-8 decoder.
Windows OFF:9eb586a6 passed a fresh Windows x64 / Python 3.13.15 Release OFF build with /W4 /WX, followed by source-package/fresh-native import with profiling absence asserted. The current source has identical native and build inputs; subsequent changes here are test-only. This is explicit source equivalence, not a newly rebuilt Windows binary or execution of the cleanup-failure cases.
Focused cleanup-failure regression — both cases passed in current-source CI: In response to the requested detach/unbind failure-injection coverage, test_fetchmany_failed_cleanup_blocks_reuse_until_cleanup_succeeds runs isolated [unbind] and [rows_fetched_ptr] cases. The first injects SQL_ERROR through the existing SQLFreeStmt_ptr. The second leaves real unbinding untouched and injects SQL_ERROR only when SQLSetStmtAttr_ptr clears SQL_ATTR_ROWS_FETCHED_PTR, exercising failure after successful unbinding. In each case, a resized fetch and a retry at the original size must return failure with no rows, retry cleanup, and leave actual plan-allocation, bind, and fetch counts unchanged. Restoring the pointer must allow recovery with the unconsumed rows and then EOF. The test also rejects terminal-retention diagnostics. This covers the observable native-return/non-reuse/recovery contract, not direct private C++ allocation/destructor proof or public exception translation.
Azure DevOps build 178187 checked out merge 21797ffd, whose complete tree c23883f3 exactly matches published head 39e22098. Both Ubuntu x86_64 / Python 3.12.3 SQL Server 2022 and SQL Server 2025 legs completed fresh Release ON native builds and the existing checkout/profiling import guard. Both exact cleanup-failure cases passed once in each leg, with zero selected skips; the three existing size/encoding/result cases also passed. Each full pytest task reported 5,559 passed, 132 skipped, 42 deselected, 2 warnings. Raw checkout/build/test logs are retained. Successful child counters and native-binary hashes were not independently exported; these results do not claim Windows execution, full-matrix completion, performance gains, or direct destructor proof. Earlier build 178178 qualified only the original single failed-unbind case at db19e637; its evidence remains separate. The withdrawn native target's 1c05d11e Windows results remain historical only and are not evidence for these cases.
The #809 integration captures intermediate retained-binding warnings into native-only per-call storage, then appends them to cursor messages outside the cleanup gate with the GIL held. Checked unbinding and original failing statuses are preserved; SQL_SUCCESS does not trigger diagnostic scans. Runtime warning/error ordering and broad fault-path qualification remain outside these focused tests.
Prior scoped correctness evidence: Linux Release OFF checks of frozen #796 revision 65081f06 and 13ad1165 passed 339 ordinary + 3 isolated tests per arm, zero skips. The same 65081f06 arm was reused for this incremental comparison; removed tests/native fixtures and ON runs were not included. Source-hash differences were confirmed as archive CRLF/LF differences, and the scoped correctness/minimality review closed for those revisions. These results do not qualify the current head.
Historical mechanism evidence only: earlier 416e54d6 / tree 99926c46 qualification included native fixtures, standalone 6 baseline / 9 candidate cases, and fetchmany(1) counts of one plan, 24 actual binds, one unbind, and 10,001 native fetches for 10,000 rows × 24 INT columns. This does not qualify the current head or establish elapsed-time gains. Baseline retained-binding counters were unavailable, not zero.
Original failures, skips, and inapplicable baseline-counter oracles remain in the qualification ledger. This is not a clean full-suite claim; mock ODBC/GIL fault tests are not live-driver concurrency proof.
Latency, no-regression, current-main, and pyodbc acceptance remain open. This is not performance/readiness signoff.
Preserve reserve-before-locking-weak-handles and release retained handles outside the child-list mutex. Expose the unchanged native-only algorithm for direct invariant coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require successful scalar NULL handling and exact describe counts. Add production-header cache, failure, concurrency, allocation and lifetime tests with active Release assertions, plus Windows/Linux/macOS CTest CI and test guidance.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use C++ streams for test-runner output, document the line-scoped allocator rule exception, and trigger native invariant tests for production integration header and binding changes. Keep production fetch code and the allocation-failure probes unchanged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Align only the reviewed fetch-buffer follow-up with dependency416.
The exact tree matches the qualified99926c46 snapshot; the allocator
translation-unit split is test-only and leaves production unchanged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove PR-only tests, native test build and CI wiring, and restore the base test guide. Inline the test-only invalidation helper and single-caller metadata setup; stop retaining unused precision/nullability fields while preserving the cache lifetime, generation, Unicode, variant and failure contracts.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Merge the frozen metadata dependency and remove PR-only test/build scaffolding.
Use direct profiled ODBC calls and the dependency child snapshot loop while
preserving retained-buffer ownership and cleanup guards.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the existing fetch-buffer reuse changes and integrate the merged
metadata optimization plus current main without rewriting branch history.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve native fetch diagnostics across retained binding setup and cleanup, without Python work under the cleanup gate.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Unix / SQL Server 2022: Python 3.12.3, x86_64, SQL 16.0.4295.3; 5 paired comparisons and 1 warmup.
Unix / SQL Server 2025: Python 3.12.3, x86_64, SQL 17.0.5005.3; 5 paired comparisons and 1 warmup.
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent.
The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes.
Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency.
Raw samples and logs are attached to the ADO run as profiler-* artifacts.
1660 std::shared_ptr<ConnectionCleanupState> cleanupState)
1661 : _type(type), _handle(rawHandle), _cleanupState(std::move(cleanupState)) {}
16621663SqlHandle::~SqlHandle() {
! 1664try {
! 1665if (_handle) {
1666SQLRETURN ret = freeHandle();
! 1667if (!SQL_SUCCEEDED(ret)) {
! 1668// A failed free leaves a live handle. Detach if possible before1669// the last plan owner applies its native-only emergency policy.1670SQLRETURN detached = detachFetchBindings();
1671 std::cerr << "mssql-python: native handle cleanup failed (" << ret
1672 << "), fetch buffer detach returned " << detached << '\n';
Lines 1693-1701
1693 }
16941695voidSqlHandle::markImplicitlyFreed() {
1696// SAFETY: Only STMT handles should be marked as implicitly freed.
! 1697// Successful SQLDisconnect frees the connection's child statements.1698// Other handle types (ENV, DBC, DESC) are NOT automatically freed by parents.1699// Calling this on wrong handle types will cause silent handle leaks.1700if (_type != SQL_HANDLE_STMT) {
1701// Log error but don't throw - we're likely in cleanup/destructor path
Replace banned formatted C output without entering Python during cleanup. Add isolated profiler regression checks for binding reuse and size, encoding, and result-set transitions.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep SQL_CHAR fixed while changing ASCII to Latin-1 so the encoding-only cache miss reaches the native fetch path.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercise real statement-owned plans with injected unbind and rows-fetched-pointer cleanup failures, runtime lifetime assertions, and successful release recovery. Keep the native target opt-in and run it in the existing Ubuntu PR jobs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Avoid std::cerr in destructor after Python shutdown
mssql_python/pybind/ddbc_bindings.cpp:1672
This destructor is explicitly documented below as a path that must not log because it can run after Python shutdown, but the new failure path writes through std::cerr. C++ iostream teardown is not guaranteed to be usable from such a destructor; use the existing non-throwing native fputs/stderr style or suppress this diagnostic.
Revert 1c05d11 after the user rejected the expanded scope. Restore the exact be219eb source tree, preserving the earlier review fixes and profiler regressions.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercise an injected SQL_UNBIND failure in the existing isolated profiler test suite. Assert that failed cleanup blocks reuse and rebinding, then verify recovery and EOF without changing production or build configuration.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Check the existing SQLFetchScroll call counter alongside plan allocation and binding counts in the single cleanup regression.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Parameterize the existing isolated cleanup regression to fail either SQL_UNBIND or clearing SQL_ATTR_ROWS_FETCHED_PTR. Preserve no-allocation, no-bind, no-fetch, retry, and recovery assertions without changing production or build configuration.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This destructor can run during interpreter/process teardown, but the new std::cerr use depends on the C++ iostream runtime and conflicts with the nearby rule that destructors must not log (ddbc_bindings.cpp:1713-1718). If cleanup fails late in shutdown, this can access torn-down iostream state and turn an already-handled cleanup failure into a crash. Use the existing C-style stderr path (or omit this diagnostic) instead of std::cerr.
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
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-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Work Item / Issue Reference
Summary
Follow-up to #796, now merged into
main. #796 reuses result-set metadata; this change reuses one statement-owned native buffer and ODBC binding plan across compatible, bounded-columnfetchmany()calls.Reuse requires the same result generation, metadata, batch size, and decoding settings. Checked cleanup protects driver-held pointers across incompatible transitions, errors, and close. Public APIs/defaults, Row/converter semantics, and MAX/LOB/
sql_variantfallback routes remain unchanged; there is no hidden prefetch.Repository scope (
30893611→39e22098): five runtime files plus regressions in the existingtests/test_025_profiler.py. There is no effective build/CI or documentation diff. Three isolated cases use the existing profiler to assert actual bind/unbind counts for a compatible hit, a size/encoding/result-set miss, resumed reuse, and EOF. One additional focused test, parameterized for failed unbinding and failed rows-fetched-pointer cleanup, covers failure propagation, blocked reuse/rebinding/fetching, and recovery. These cases require a profiling-enabled native build and live SQL; unavailable profiling is explicitly skipped, not reported as zero calls. The failure-injection cases also skip Windows because its ODBC function-pointer globals are not exported there.Scope correction:
ad0a8550reverted the four-file, 308-line cleanup-test increment1c05d11eafter the user rejected its size. The native test target, CMake/CI wiring, and related documentation remain removed.ad0a8550restored the completebe219eb3source tree; the follow-up throughdb19e637added only one 93-line test in the existing test module.39e22098parameterizes that same test for the second cleanup-failure boundary (+26/-10 in that file only). There are no additional production, build, CI, or documentation changes. The earlier DevSkim fixes and three profiler regressions remain; history was preserved through a normal revert, not rewritten.flowchart LR subgraph "Before (#796)" B["Each fetchmany"] --> S["Allocate + bind"] --> F["Fetch + unbind"] end subgraph After A["First compatible call"] --> P["Owned buffers + bindings"] P --> R["Reuse on subsequent fetchmany calls"] R --> D["Detach at EOF / transition"] endMechanism and correctness
Actual bind/unbind/attribute-call timers and plan-allocation profiling are preserved. The auxiliary value-buffer-growth timer was removed; its absence is not zero allocation work.
The source preserves the main merges through
30893611and the two DevSkim fixes in9eb586a6, which replace formatted C calls with native-only output. The reported cache/cleanup lock inversion was checked against the function-local mutex lifetimes: cache clearing returns and releases its lock before the cleanup gate is acquired; no teardown reordering was needed.Prior SQL counter validation and source equivalence: Azure DevOps build 178157 checked out merge
e3dc03c0, whose complete treee29a5927matches bothbe219eb3andad0a8550. Both Ubuntu x86_64 / Python 3.12.3 SQL Server 2022 and SQL Server 2025 legs completed fresh Release ON native builds and the existing checkout/profiling import guard. Each leg passed all three size/encoding/result regression cases, with zero selected skips. Each full pytest task reported 5,557 passed, 132 skipped, 42 deselected. These results remain attributed to that CI run and do not qualify the subsequently added cleanup-failure cases. Successful raw counters and native-binary hashes were not independently exported; this is not full-matrix qualification.The original
9eb586a6CI run exposed an unsupported UTF-16 alias in the encoding test before its transition fetch.be219eb3corrected only two test-input lines to supported ASCII and Latin-1 settings with SQL_CHAR fixed; the original failure is retained in the evidence. This tests decoding-configuration invalidation, not a change in Linux's effective UTF-8 decoder.Windows OFF:
9eb586a6passed a fresh Windows x64 / Python 3.13.15 Release OFF build with/W4 /WX, followed by source-package/fresh-native import with profiling absence asserted. The current source has identical native and build inputs; subsequent changes here are test-only. This is explicit source equivalence, not a newly rebuilt Windows binary or execution of the cleanup-failure cases.Focused cleanup-failure regression — both cases passed in current-source CI: In response to the requested detach/unbind failure-injection coverage,
test_fetchmany_failed_cleanup_blocks_reuse_until_cleanup_succeedsruns isolated[unbind]and[rows_fetched_ptr]cases. The first injectsSQL_ERRORthrough the existingSQLFreeStmt_ptr. The second leaves real unbinding untouched and injectsSQL_ERRORonly whenSQLSetStmtAttr_ptrclearsSQL_ATTR_ROWS_FETCHED_PTR, exercising failure after successful unbinding. In each case, a resized fetch and a retry at the original size must return failure with no rows, retry cleanup, and leave actual plan-allocation, bind, and fetch counts unchanged. Restoring the pointer must allow recovery with the unconsumed rows and then EOF. The test also rejects terminal-retention diagnostics. This covers the observable native-return/non-reuse/recovery contract, not direct private C++ allocation/destructor proof or public exception translation.Azure DevOps build 178187 checked out merge
21797ffd, whose complete treec23883f3exactly matches published head39e22098. Both Ubuntu x86_64 / Python 3.12.3 SQL Server 2022 and SQL Server 2025 legs completed fresh Release ON native builds and the existing checkout/profiling import guard. Both exact cleanup-failure cases passed once in each leg, with zero selected skips; the three existing size/encoding/result cases also passed. Each full pytest task reported 5,559 passed, 132 skipped, 42 deselected, 2 warnings. Raw checkout/build/test logs are retained. Successful child counters and native-binary hashes were not independently exported; these results do not claim Windows execution, full-matrix completion, performance gains, or direct destructor proof. Earlier build178178qualified only the original single failed-unbind case atdb19e637; its evidence remains separate. The withdrawn native target's1c05d11eWindows results remain historical only and are not evidence for these cases.The #809 integration captures intermediate retained-binding warnings into native-only per-call storage, then appends them to cursor messages outside the cleanup gate with the GIL held. Checked unbinding and original failing statuses are preserved;
SQL_SUCCESSdoes not trigger diagnostic scans. Runtime warning/error ordering and broad fault-path qualification remain outside these focused tests.Prior scoped correctness evidence: Linux Release OFF checks of frozen #796 revision
65081f06and13ad1165passed 339 ordinary + 3 isolated tests per arm, zero skips. The same65081f06arm was reused for this incremental comparison; removed tests/native fixtures and ON runs were not included. Source-hash differences were confirmed as archive CRLF/LF differences, and the scoped correctness/minimality review closed for those revisions. These results do not qualify the current head.Historical mechanism evidence only: earlier
416e54d6/ tree99926c46qualification included native fixtures, standalone 6 baseline / 9 candidate cases, andfetchmany(1)counts of one plan, 24 actual binds, one unbind, and 10,001 native fetches for 10,000 rows × 24 INT columns. This does not qualify the current head or establish elapsed-time gains. Baseline retained-binding counters were unavailable, not zero.Original failures, skips, and inapplicable baseline-counter oracles remain in the qualification ledger. This is not a clean full-suite claim; mock ODBC/GIL fault tests are not live-driver concurrency proof.
Latency, no-regression, current-main, and pyodbc acceptance remain open. This is not performance/readiness signoff.