database/sql: document that Rows.Close must be called

[adonovan]

Failure to call Rows.Close leads to a resource leak. Although the usage example calls defer rows.Close(), it's easy for a caller of, say, QueryContext not to realize that the Rows result is Closeable and to forget to call it.

The documentation of Rows, and any function that returns a Rows, should state that the caller is responsible for calling Close.

Also, the examples suggest that errors from Close may be ignored for read-only queries but should not be ignored for updates. In this respect the database is analogous to a readable or writable file; this too is worth pointing out explicitly.

See also #33938.

[gopherbot]

Change https://golang.org/cl/333989 mentions this issue: database/sql: document that *Rows must be closed after Query

[hardikmodha]

While using rows.Next, do we need to close it explicitly? e.g. I can see in the following code that it gets closed automatically if there are no more result sets.

https://cs.opensource.google/go/go/+/refs/tags/go1.21.1:src/database/sql/sql.go;drc=894d24d617bb72d6e1bed7b143f9f7a0ac16b844;l=3033

Scenario 1: Not returning any error from for loop

for rows.Next() {
  // no possibility of any abnormal exit here. 
}

Scenario 2: Returning from for loop due to some error while scanning the row

for rows.Next() {
   err := rows.Scan()

   if err != nil {
         return err
   }
}

My understanding is that,

In scenario 1, would there be any resource leak cause we are not explicitly closing the rows?

In scenario 2, there would be resource leak as we are not closing the rows and rows.Next() won't be able to close the rows.

But as a good practice, we should close it explicitly.

Can someone confirm if my understanding is correct?

Thanks!

[adonovan]

My understanding is that,
In scenario 1, would there be any resource leak cause we are not explicitly closing the rows?
In scenario 2, there would be resource leak as we are not closing the rows and rows.Next() won't be able to close the rows.
But as a good practice, we should close it explicitly.
Can someone confirm if my understanding is correct?

Search (the anchor is broken) for the section named "The race" in https://github.blog/2020-05-20-three-bugs-in-the-go-mysql-driver for an interesting story in which not calling Close led to a very subtle bug that would (IIUC) still have affected users even in loops of the first kind. The post observes that almost no-one bothers to call Close correctly, so arguably the driver implementor should be more defensive; the documentation is not very explicit. The driver has since been changed to be more defensive, but I think it would be imprudent to assume that scenario 1 is safe.

[ambareesh-jayakumari]

Got here through search, and later found this excellent tutorial: http://go-database-sql.org/index.html

The above mentioned surprise is mentioned in http://go-database-sql.org/errors.html.