Conversation
9420ccc to
40e8233
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The test does not validate explicit result-set drainage and lacks failure-safe cleanup.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds regression coverage for stored procedures combining result sets and OUT parameters.
Changes:
- Verifies internal result retrieval.
- Fetches OUT parameters after advancing result sets.
File summaries
| File | Description |
|---|---|
pymysql/tests/test_issues.py |
Adds the callproc regression test. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
40e8233 to
70a0d06
Compare
|
addressed Copilot feedback by adding try/finally cleanup and explicit nextset() assertions. |
|
Hi @PyMySQL maintainers, just checking in on this when you have a moment! All CI checks are passing cleanly. Please let me know if you would like any adjustments to the implementation or additional test cases added. Happy to make updates! |
Summary
Adds a dedicated test case to verify
cursor.callproc()behavior when executing stored procedures that return internal result sets alongsideOUTparameters.Details
When a MySQL stored procedure contains internal queries (e.g.,
SELECTstatements) and updatesOUTparameters, the server returns multiple result set packets over the socket.This test ensures that:
SELECTresult sets remain accessible viacursor.fetchall().cursor.nextset()allows session variables (@_procname_n) to be fetched reliably via subsequentSELECTqueries without socket desynchronization.Testing
test_callproc_with_multi_resultset_and_out_paramsunderpymysql/tests/test_issues.py.pytest pymysql/tests/) to ensure zero regressions across standardcallprocusage.