Skip to content

client: add Stmt.ExecuteProcedureMultiResults for multi-result stored procedures#1165

Merged
lance6716 merged 10 commits into
go-mysql-org:masterfrom
nzin-alayacare:feat/stmt-execute-procedure-multi-results
Jul 9, 2026
Merged

client: add Stmt.ExecuteProcedureMultiResults for multi-result stored procedures#1165
lance6716 merged 10 commits into
go-mysql-org:masterfrom
nzin-alayacare:feat/stmt-execute-procedure-multi-results

Conversation

@nzin-alayacare

Copy link
Copy Markdown
Contributor

Summary

Add (*Stmt).ExecuteProcedureMultiResults and the StmtProcedureMultiResultForward callback type to client/stmt.go.

Closes #1162

Motivation

(*Stmt).Execute reads a single result via readResult(true) and stops. When a stored procedure returns multiple result sets (e.g., a CALL with OUT parameters or multiple SELECT statements inside), the server sets SERVER_MORE_RESULTS_EXISTS in intermediate result statuses. Calling Execute in this case leaves subsequent packets buffered on the connection, corrupting its state.

The text-protocol equivalent Conn.ExecuteMultiple already handles this correctly by looping until the flag is clear. This PR adds the same capability for prepared statements (COM_STMT_EXECUTE), which is the path taken by Connector/J and other JDBC drivers for CallableStatement.

New API

// StmtProcedureMultiResultForward is called once per logical result returned by
// COM_STMT_EXECUTE. When err is non-nil, res is nil.
type StmtProcedureMultiResultForward func(res *mysql.Result, err error) error

// ExecuteProcedureMultiResults runs COM_STMT_EXECUTE and reads every response
// until SERVER_MORE_RESULTS_EXISTS is clear.  forward is called for each
// result; the loop keeps draining even after forward returns non-nil so that
// unread packets do not corrupt the connection state.
func (s *Stmt) ExecuteProcedureMultiResults(
    args []any,
    forward StmtProcedureMultiResultForward,
) (*mysql.Result, error)

Design notes

  • Drain-on-error: the loop does not stop when forward returns an error because leaving unread results on the wire would corrupt the connection. Instead the forward error is saved and returned only after all results have been consumed — identical to ExecuteMultiple semantics.
  • Synthetic return value: a StreamingMultiple / StreamingDone result is returned on success so callers can pass it to server.Conn.WriteValue without additional handling.
  • Nil check on callback: returns an immediate error rather than panic.

Notes

Made with Cursor

… procedures

ExecuteProcedureMultiResults runs COM_STMT_EXECUTE and drains all
logical results until SERVER_MORE_RESULTS_EXISTS is clear. A
caller-supplied StmtProcedureMultiResultForward callback is invoked
for each result in arrival order.

The loop continues draining the server response even after forward
returns non-nil, matching ExecuteMultiple semantics so that unread
packets cannot corrupt the connection state.

This is the prepared-statement equivalent of ExecuteMultiple. It is
needed for CALL statements with OUT parameters or multiple SELECT
result sets, where Connector/J (and other JDBC drivers) use
COM_STMT_EXECUTE and expect every result set to be forwarded.

Closes go-mysql-org#1162

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the ExecuteProcedureMultiResults method to the Stmt struct in client/stmt.go to support executing stored procedures that return multiple result sets. The review feedback suggests improving the API's usability and consistency by making the args parameter variadic and placing the forward callback first, as well as initializing the synthetic resultset with 0 fields instead of 1.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread client/stmt.go Outdated
Comment thread client/stmt.go Outdated
nzin-alayacare and others added 4 commits July 2, 2026 10:40
…ltiResults

Place the forward callback first with variadic args for consistency with
Stmt.Execute and Stmt.ExecuteSelectStreaming. Initialize the synthetic
streaming resultset with zero fields to match ExecuteMultiple.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add a nil-callback unit test and a clientTestSuite integration test that
creates a stored procedure returning two result sets. Negotiate
CLIENT_MULTI_RESULTS and CLIENT_PS_MULTI_RESULTS in SetupSuite.

Co-authored-by: Cursor <cursoragent@cursor.com>
CLIENT_MULTI_RESULTS and CLIENT_PS_MULTI_RESULTS are already included in
the client's default capability set (client/auth.go) and are negotiated
automatically during the initial handshake. Setting them again in
SetupSuite was redundant and caused TestConn_SetCapability to fail because
that test asserts those capabilities start unset on the shared connection.

Co-authored-by: Cursor <cursoragent@cursor.com>
The shared suite connection does not set CLIENT_MULTI_RESULTS or
CLIENT_PS_MULTI_RESULTS (they are optional capabilities), so calling a
stored procedure through it produced ERROR 1312. Open a per-test
connection that explicitly enables both capabilities, which is the
correct way to test ExecuteProcedureMultiResults without disturbing the
shared connection used by TestConn_SetCapability and other suite tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread client/client_test.go Outdated
Comment thread client/stmt.go Outdated
nzin-alayacare and others added 2 commits July 8, 2026 12:35
Remove the synthetic NewResultset(0) return value; callers that forward
results via the callback do not need a dummy *mysql.Result. Matches
ExecuteSelectStreaming semantics and addresses review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>
Rename TestStmt_ExecuteProcedureMultiResults to TestStmt_ProcedureMultiResult
and TestExecuteProcedureMultiResults_NilCallback to
TestStmtProcedureMultiResultNilForward, consistent with other TestStmt_* names.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread client/client_test.go Outdated
nzin-alayacare and others added 2 commits July 9, 2026 02:37
…esult

Match the naming style used in conn_test (e.g. TestExecuteMultiple) per
review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>

@lance6716 lance6716 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest lgtm

Comment thread client/stmt.go Outdated
Clarify that a nil return from forward means the result or error was
handled and is not propagated. Simplify the final return to
`return forwardErr` per review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lance6716 lance6716 merged commit 8a3d353 into go-mysql-org:master Jul 9, 2026
20 checks passed
dbnski pushed a commit to dbnski/go-mysql that referenced this pull request Jul 10, 2026
… procedures (go-mysql-org#1165)

* client: add Stmt.ExecuteProcedureMultiResults for multi-result stored procedures

ExecuteProcedureMultiResults runs COM_STMT_EXECUTE and drains all
logical results until SERVER_MORE_RESULTS_EXISTS is clear. A
caller-supplied StmtProcedureMultiResultForward callback is invoked
for each result in arrival order.

The loop continues draining the server response even after forward
returns non-nil, matching ExecuteMultiple semantics so that unread
packets cannot corrupt the connection state.

This is the prepared-statement equivalent of ExecuteMultiple. It is
needed for CALL statements with OUT parameters or multiple SELECT
result sets, where Connector/J (and other JDBC drivers) use
COM_STMT_EXECUTE and expect every result set to be forwarded.

Closes go-mysql-org#1162

Co-authored-by: Cursor <cursoragent@cursor.com>

* client: variadic signature and NewResultset(0) for ExecuteProcedureMultiResults

Place the forward callback first with variadic args for consistency with
Stmt.Execute and Stmt.ExecuteSelectStreaming. Initialize the synthetic
streaming resultset with zero fields to match ExecuteMultiple.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: add unit and integration tests for ExecuteProcedureMultiResults

Add a nil-callback unit test and a clientTestSuite integration test that
creates a stored procedure returning two result sets. Negotiate
CLIENT_MULTI_RESULTS and CLIENT_PS_MULTI_RESULTS in SetupSuite.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: remove redundant SetCapability calls from SetupSuite

CLIENT_MULTI_RESULTS and CLIENT_PS_MULTI_RESULTS are already included in
the client's default capability set (client/auth.go) and are negotiated
automatically during the initial handshake. Setting them again in
SetupSuite was redundant and caused TestConn_SetCapability to fail because
that test asserts those capabilities start unset on the shared connection.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: use dedicated connection with multi-result caps in procedure test

The shared suite connection does not set CLIENT_MULTI_RESULTS or
CLIENT_PS_MULTI_RESULTS (they are optional capabilities), so calling a
stored procedure through it produced ERROR 1312. Open a per-test
connection that explicitly enables both capabilities, which is the
correct way to test ExecuteProcedureMultiResults without disturbing the
shared connection used by TestConn_SetCapability and other suite tests.

Co-authored-by: Cursor <cursoragent@cursor.com>

* client: return error only from ExecuteProcedureMultiResults

Remove the synthetic NewResultset(0) return value; callers that forward
results via the callback do not need a dummy *mysql.Result. Matches
ExecuteSelectStreaming semantics and addresses review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: rename procedure multi-result tests to match naming style

Rename TestStmt_ExecuteProcedureMultiResults to TestStmt_ProcedureMultiResult
and TestExecuteProcedureMultiResults_NilCallback to
TestStmtProcedureMultiResultNilForward, consistent with other TestStmt_* names.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: rename TestStmt_ProcedureMultiResult to TestStmtProcedureMultiResult

Match the naming style used in conn_test (e.g. TestExecuteMultiple) per
review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>

* client: document forward error handling and simplify return

Clarify that a nil return from forward means the result or error was
handled and is not propagated. Simplify the final return to
`return forwardErr` per review feedback on PR go-mysql-org#1165.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: lance6716 <lance6716@gmail.com>
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.

client: add Stmt.ExecuteProcedureMultiResults for draining stored procedure result sets

2 participants