I need to make SQL queries to database in the loop:
for rows.Next() {
fields, err := db.Query(.....)
if err != nil {
// ...
}
defer fields.Close()
// do something with `fields`
}
What will be better: leave all as is or move defer after loop:
for rows.Next() {
fields, err := db.Query(.....)
if err != nil {
// ...
}
// do something with `fields`
}
defer fields.Close()
Or something else ?
Execution of a deferred function is not only delayed, deferred to the moment the surrounding function returns, it is also executed even if the enclosing function terminates abruptly, e.g. panics. Spec: Defer statements:
A "defer" statement invokes a function whose execution is deferred to the moment the surrounding function returns, either because the surrounding function executed a return statement, reached the end of its function body, or because the corresponding goroutine is panicking.
Whenever you create a value or a resource that provides means to properly close it / dispose of it, you should always use a defer statement to make sure it is released even if your other code panics to prevent leaking memory or other system resources.
It's true that if you're allocating resources in a loop you should not simply use defer, as then releasing resources will not happen as early as it could and should (at the end of each iteration), only after the for statement (only after all iterations).
What you should do is that if you have a snippet that allocates such resources, wrap it in a function –either an anonymous or a named function–, and in that function you may use defer, and resources will be freed as soon as they are no longer needed, and what's important is that even if there is a bug in your code which may panic.
Example:
for rows.Next() {
func() {
fields, err := db.Query(...)
if err != nil {
// Handle error and return
return
}
defer fields.Close()
// do something with `fields`
}()
}
Or if put in a named function:
func foo(rs *db.Rows) {
fields, err := db.Query(...)
if err != nil {
// Handle error and return
return
}
defer fields.Close()
// do something with `fields`
}
And calling it:
for rows.Next() {
foo(rs)
}
Also if you'd want to terminate on the first error, you could return the error from foo():
func foo(rs *db.Rows) error {
fields, err := db.Query(...)
if err != nil {
return fmt.Errorf("db.Query error: %w", err)
}
defer fields.Close()
// do something with `fields`
return nil
}
And calling it:
for rows.Next() {
if err := foo(rs); err != nil {
// Handle error and return
return
}
}
Also note that Rows.Close() returns an error which when called using defer is discarded. If we want to check the returned error, we can use an anonymous function like this:
func foo(rs *db.Rows) (err error) {
fields, err := db.Query(...)
if err != nil {
return fmt.Errorf("db.Query error: %w", err)
}
defer func() {
if err = fields.Close(); err != nil {
err = fmt.Errorf("Rows.Close() error: %w", err)
}
}()
// do something with `fields`
return nil
}
The whole point of defer is that it does not execute until the function returns, so the appropriate place to put it would be immediately after the resource you want to close is opened. However, since you're creating the resource inside the loop, you should not use defer at all - otherwise, you're not going to close any of the resources created inside the loop until the function exits, so they'll pile up until then. Instead, you should close them at the end of each loop iteration, without defer:
for rows.Next() {
fields, err := db.Query(.....)
if err != nil {
// ...
}
// do something with `fields`
fields.Close()
}
You can construct a local function to solve this problem
for i := 0; i < 5; i++ {
func() {
f, err := os.Open("/path/to/file")
if err != nil {
log.Fatal(err)
} else {
defer f.Close()
}
}()
}
