Goのdatabase/sqlでRows処理の責務漏れをどうレビューするか

Goで database/sql を使うと、QueryContext して rows.Next() を回す実装はすぐに書ける。
ただし、Rowsのライフサイクルとエラー確認が曖昧なコードは、レビューで見落とすと本番障害に直結しやすい。

特に危ないのは次のような実装だ。

rows.Close() がない、または呼び出し責務が読めない
for rows.Next() のあとに rows.Err() を確認していない
Scan() エラー時に途中まで詰めたsliceの扱いが曖昧
DB取得結果をそのままAPI返却構造へ流し、空配列やnilの契約が崩れる

この記事ではSQLの書き方ではなく、
取得処理の終了条件と失敗時の返却契約をレビューでどう読むかに絞って整理する。

まず止めたい実装

Rows責務が漏れている実装

func ListOrders(ctx context.Context, db *sql.DB, userID string) ([]Order, error) {
    rows, err := db.QueryContext(ctx, `
        SELECT id, status, total
        FROM orders
        WHERE user_id = ?
        ORDER BY created_at DESC
    `, userID)
    if err != nil {
        return nil, err
    }
@Reviewer
`rows.Close()` がありません。QueryContextで取得したRowsの所有者がこの関数なら、成功・失敗に関わらずCloseされる構造にしてください。
    var orders []Order
    for rows.Next() {
        var order Order
        if err := rows.Scan(&order.ID, &order.Status, &order.Total); err != nil {
            return orders, err
        }
@Reviewer
Scan失敗時に途中まで詰めたordersを返しています。呼び出し側が部分結果を利用してよい契約なのか不明です。通常はnilとerrorを返し、失敗時の契約を明確にしてください。
        orders = append(orders, order)
    }

    return orders, nil
@Reviewer
rows.Next()終了後の`rows.Err()`確認がありません。ネットワーク断や読み取り途中のエラーが成功扱いになる可能性があります。
}

このコードは、テストデータが少ないうちは正常に見える。
しかしレビューで見るべきなのは「1件読めるか」ではない。
途中で失敗したときに、接続・エラー・返却値の責務が崩れないかである。

なぜ危ないのか

rows.Next() は、行がなくなった場合も、読み取り途中でエラーになった場合も false を返す。
そのためループ後に rows.Err() を確認しないと、次のような状況が成功扱いになり得る。

DB接続が途中で切れた
コンテキストがキャンセルされた
ドライバ側で読み取りエラーが起きた
一部の行だけ読めた状態で処理が終わった

さらに rows.Close() が抜けると、接続プールの枯渇や次のクエリの遅延につながる。
レビューでは、QueryContext の行だけでなく、そのRowsを誰が閉じ、どこで読み取り完了を確定するかまで追う必要がある。

レビューで見たい3つの判断線

1. `QueryContext` の直後にClose予約があるか

Rowsの所有者がその関数であれば、err 確認後すぐに defer rows.Close() がある形が読みやすい。

Comment

@Reviewer: `QueryContext` 成功後に `rows.Close()` が予約されていません。Rowsの所有者がこの関数なら、取得直後に `defer rows.Close()` を置いてリソース解放責務を固定してください。

Closeが別関数に委譲されている場合でも、レビューでは「どこが所有者か」を確認する。
曖昧な所有権は、将来の分岐追加で漏れやすい。

2. ループ終了後に `rows.Err()` を確認しているか

rows.Next() の終了は成功とは限らない。
ループ後の rows.Err() は、複数行取得レビューの基本確認点になる。

Comment

@Reviewer: `rows.Next()` の後に `rows.Err()` を確認してください。途中読み取りエラーと正常終了を区別できないため、部分結果が成功扱いになる可能性があります。

Scan() エラーだけを見ていても不十分だ。
行の読み出し自体に失敗した場合は、rows.Err() 側に現れる。

3. 失敗時に部分結果を返す契約になっていないか

業務APIでは、DB読み取りに失敗したときに部分結果を返す設計はかなり慎重に扱うべきだ。

Comment

@Reviewer: Scanエラー時に途中まで取得したsliceを返しています。部分結果を許可する仕様でなければ、失敗時は `nil, err` に統一し、呼び出し側が誤って不完全なデータを表示しないようにしてください。

部分結果が必要なケースもある。
ただしその場合は、戻り値型やコメントで「部分成功」を表現し、通常の ([]T, error) に混ぜないほうがよい。

改善例

最低限、次のようにRowsのライフサイクルを関数内で閉じるとレビューしやすい。

Rows責務を固定した実装

func ListOrders(ctx context.Context, db *sql.DB, userID string) ([]Order, error) {
    rows, err := db.QueryContext(ctx, `
        SELECT id, status, total
        FROM orders
        WHERE user_id = ?
        ORDER BY created_at DESC
    `, userID)
    if err != nil {
        return nil, fmt.Errorf("query orders: %w", err)
    }
    defer rows.Close()

    orders := make([]Order, 0)
    for rows.Next() {
        var order Order
        if err := rows.Scan(&order.ID, &order.Status, &order.Total); err != nil {
            return nil, fmt.Errorf("scan order: %w", err)
        }

        orders = append(orders, order)
    }

    if err := rows.Err(); err != nil {
        return nil, fmt.Errorf("iterate orders: %w", err)
    }

    return orders, nil
}

この構造では、レビューアーが次をすぐ確認できる。

Rowsの解放責務がこの関数にある
読み取り途中のエラーが成功扱いにならない
失敗時に部分結果を返さない
呼び出し元へ渡すエラー文脈が残っている

`Close` だけでは成功判定にならない

defer rows.Close() があるだけで安心してはいけない。
Close はリソース解放であり、読み取り完了の成功判定は rows.Err() で行う。

CloseはあるがErr確認がない実装

func FindItems(ctx context.Context, db *sql.DB) ([]Item, error) {
    rows, err := db.QueryContext(ctx, "SELECT id, name FROM items")
    if err != nil {
        return nil, err
    }
    defer rows.Close()

    var items []Item
    for rows.Next() {
        var item Item
        if err := rows.Scan(&item.ID, &item.Name); err != nil {
            return nil, err
        }
        items = append(items, item)
    }

    return items, nil
}

Comment

@Reviewer: `rows.Close()` はありますが、読み取り途中の失敗を検知する `rows.Err()` がありません。ループ終了後に確認し、正常終了と異常終了を分けてください。

レビュー観点チェックリスト

database/sql Rowsレビューの確認項目

QueryContext 成功後に defer rows.Close() があるか
Rowsの所有者が関数・呼び出し元・ヘルパーのどこか明確か
Scan() エラー時に部分結果を返していないか
rows.Next() 後に rows.Err() を確認しているか
エラーに query / scan / iterate の文脈が付いているか
空結果、失敗、部分成功の返却契約が混ざっていないか

まとめ

database/sql の複数行取得レビューでは、SQLの妥当性だけを見ると危険を見落とす。
レビューアーが見るべきなのは、Rowsの所有、読み取り完了、失敗時返却の3点が構造として説明できるかである。

特に rows.Close() と rows.Err() は、単なる作法ではない。
DB接続と部分結果の扱いを安定させるための、レビュー上の最低限の判断線として確認したい。

SNSリンクボタン

Goのdatabase/sqlでRows処理の責務漏れをどうレビューするか

前後に公開されたマニュアル

SNSリンクボタン

サイト内検索フォーム