Probable bug in database/sql

[Marco De Zio]

It seems to me that the second flag returned by Rows.nextLocked is wrongly interpreted by Rows.Next as it sets rs.hitEOF = true when ok is false; also Rows.nextLocked should set ok to true in some of the cases where right now returns false.

Reference in code: https://github.com/golang/go/blob/507d1b22f4b58ac68841582d0c2c0ab6b20e5a98/src/database/sql/sql.go#L3027

Am I wrong ?

[Ian Lance Taylor]

The "ok" result of nextLocked is intended to report whether, when the
method returns, the Rows is pointing at valid data. The "doClose"
result reports whether the Rows should be closed. Given that
understanding, the behavior of Next looks reasonable to me.

As it happens when the current implementation returns "doClose" as
true, it always returns "ok" as false. So the test "if doClose &&
!ok" is not strictly necessary; it could be replaced with "if
doClose". But the current code doesn't seem wrong to me.

Can you show a code sequence that seems buggy to you?

Ian

[Marco De Zio]

Thank you for the quick response!

Let me elaborate a bit: my doubt arose from the hitEOF doc string which says that it's expected to be true when there are no more rows AND no errors where encountered; within Next, hitEOF is set to true when nextLocked's ok is false. That seems misleading to me as it's set to true even if "Rows is NOT pointing to a valid data" (or whatever error the driver's Next may return).

Now that I've also looked a bit more into it I can see that it's not really buggy as Rows.Err will still report the right error and hitEOF is used only for the closing context error, but that code still smell bad to me due to that description.