Open raxod502-plaid opened 5 months ago
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:
Query
/QueryRow
is expected to always return at least one RowSet.Exec
is like a Query
/QueryRow
whose RowSets are discarded (but has other properties).Correct usage:
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.SELECT
, INSERT .. RETURNING
, etc.), the correct expectation to be set is WillReturnRows(mock.NewRows(colNames))
. This tells the mock that one RowSet is expected.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:
const myQuery := `
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:
mock.ExpectQuery(myQuery).WithoutArgs().WillReturnRows(
mock.NewRows([]string{"disabled"}),
mock.NewRows(nil),
)
Conclusions:
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