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
Hi @raxod502-plaid! Thank you for your report. You are right that it shouldn't panic, that's a bug. It is also true that Exec would be preferable for executing any SQL that will never return any rows, like TRUNCATE.
Quick facts:
A Query/QueryRow is expected to always return at least one RowSet.
An Exec is like a Query/QueryRow whose RowSets are discarded (but has other properties).
The panic happens because it tries to look for the first RowSet and the code doesn't check that there is none. This is the bug you describe.
Correct usage:
The recommendation for your code is to use Exec as you found out. That's not a workaround, your code will benefit from explicitly stating that you are not expecting any RowSets to be returned, and additionally you may get a sql.Result (with rows affected and last inserted ID) if your driver supports it and if it makes sense for the operation.
For code that could potentially return rows (like a SELECT, INSERT .. RETURNING, etc.), the correct expectation to be set is WillReturnRows(mock.NewRows(colNames)). This tells the mock that one RowSet is expected.
It is valid to use WillReturnRows(mock.NewRows(nil)), because it states that you will expect exactly one RowSet with no columns, which is equivalent to an Exec discarding the sql.Result.
Consider a more complex example:
constmyQuery:=` SELECT disabled FROM users WHERE id = ?; DELETE FROM logins WHERE created_at < ?;`tx.QueryContext(ctx, myQuery, theID, theTimestamp)
That's a query returning two RowSets, the first one with a single column, and the second with none. A test expectation for it would be like:
sqlmock should not panic this way if WillReturnRows is not passed any arguments. Instead, it should provide a help message explaining that it expects at least one (non-nil) argument to WillReturnRows. It should also not panic if NextResultSet is called and there is none provided, but instead fail the test stating that a call to that method was not expected (didn't check if that's already in place, just saying).
Operating system and Go Version
macOS 14.4.1, Go 1.20
Issue
When using
db.Query
to perform a query that does not return any rows, sqlmock panics instead of performing a no-op as other database drivers do.A workaround is to use
db.Exec
instead so that there is no rows object to close, but the sqlmock behavior is incorrect.Reproduction steps
Expected Result
No error (or, at worst, an error returned from
rows.Close
)Actual Result
The text was updated successfully, but these errors were encountered: