Keyboard shortcuts

Press or to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 12433] ファイルの概要

このコミットは、Go言語の database/sql パッケージにおける、プリペアドステートメントの二重準備(double-Prepare)を引き起こすタイポバグを修正するものです。具体的には、データベース接続の再利用ロジックにおける変数参照の誤りを修正し、それによって不要な Prepare コールが発行される問題を解決しています。

コミット

commit 48eacd90a8ad54baf8c8037cb8d753e31e2d4bfd
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue Mar 6 14:10:58 2012 -0800

    database/sql: fix typo bug resulting in double-Prepare
    
    Bug reported by Blake Mizerany found while writing
    his new Postgres driver.
    
    R=golang-dev, blake.mizerany
    CC=golang-dev
    https://golang.org/cl/5754057

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/48eacd90a8ad54baf8c8037cb8d753e31e2d4bfd

元コミット内容

    database/sql: fix typo bug resulting in double-Prepare
    
    Bug reported by Blake Mizerany found while writing
    his new Postgres driver.
    
    R=golang-dev, blake.mizerany
    CC=golang-dev
    https://golang.org/cl/5754057

変更の背景

この変更は、Go言語の標準ライブラリである database/sql パッケージにおいて発見されたバグを修正するために行われました。このバグは、データベースのプリペアドステートメント(Prepared Statement)が意図せず二重に準備(Prepare)されてしまうというものでした。

バグは、PostgreSQLドライバーを開発していたBlake Mizerany氏によって報告されました。データベースドライバーは、database/sql パッケージが提供するインターフェースを実装することで、Goアプリケーションから様々なデータベースにアクセスできるようにします。このような低レベルのドライバー開発中に、database/sql パッケージの内部ロジックにおける非効率性や誤った挙動が露呈することがあります。

プリペアドステートメントの二重準備は、データベースサーバーへの不要な負荷、ネットワークトラフィックの増加、そしてアプリケーションのパフォーマンス低下を引き起こす可能性があります。特に、頻繁に実行されるクエリに対してこの問題が発生すると、その影響は顕著になります。このコミットは、このような非効率性を排除し、database/sql パッケージの堅牢性とパフォーマンスを向上させることを目的としています。

前提知識の解説

Go言語の database/sql パッケージ

database/sql は、Go言語の標準ライブラリに含まれるパッケージで、SQLデータベースへの汎用的なインターフェースを提供します。このパッケージ自体は特定のデータベースドライバーを含まず、driver インターフェースを定義しています。各データベース(PostgreSQL, MySQL, SQLiteなど)に対応するドライバーは、この driver インターフェースを実装し、database/sql パッケージに登録されます。

主な機能は以下の通りです。

  • DB オブジェクト: データベースへの接続プールを管理します。
  • Conn オブジェクト: データベースへの単一の接続を表します。
  • Stmt オブジェクト: プリペアドステートメントを表します。
  • QueryExec: クエリの実行と結果の取得を行います。

プリペアドステートメント (Prepared Statement)

プリペアドステートメントは、SQLクエリをデータベースサーバーに事前に準備(コンパイル)させておく機能です。これにより、以下のような利点があります。

  1. パフォーマンスの向上: 同じ構造のクエリを複数回実行する場合、毎回クエリを解析・コンパイルするオーバーヘッドがなくなります。
  2. SQLインジェクション攻撃の防止: パラメータをプレースホルダーとして渡し、データベースがそれらをデータとして扱うため、悪意のある入力によるSQLインジェクションを防ぐことができます。

database/sql パッケージでは、DB.Prepare() メソッドや Stmt オブジェクトを通じてプリペアドステートメントを扱います。理想的には、一度 Prepare されたステートメントは、その後の複数回の実行で再利用されるべきです。

データベース接続プール (Connection Pooling)

データベースへの接続は、確立にコストがかかる操作です。そのため、多くのアプリケーションでは、データベース接続プールを使用して接続を再利用します。接続プールは、事前に確立されたデータベース接続のセットを保持し、必要に応じてアプリケーションに提供します。これにより、接続の確立・切断のオーバーヘッドを削減し、アプリケーションの応答性を向上させます。

database/sql パッケージの DB オブジェクトは、内部的に接続プールを管理しています。クエリが実行される際、プールから利用可能な接続が取得され、クエリの実行後にプールに返却されます。プリペアドステートメントも、通常は特定の接続に関連付けられて管理されます。

技術的詳細

このバグは、database/sql パッケージがプリペアドステートメントを管理し、データベース接続を再利用するロジックの内部に潜んでいました。

database/sql パッケージでは、Stmt オブジェクトがプリペアドステートメントを表します。この Stmt オブジェクトは、複数のデータベース接続(driver.Conn)上で再利用される可能性があります。Stmt は、内部的に connStmt という構造体のリスト s.css を保持しており、これは各接続とそれに対応するプリペアドステートメント(driver.Stmt)のペアを管理します。

クエリが実行される際、Stmt はまず、現在利用可能な接続プール内の接続の中から、既にこの Stmt に対応するプリペアドステートメントが準備されている接続を探します。もし見つかれば、その既存のプリペアドステートメントを再利用します。見つからなければ、新しい接続を取得し、その接続上で Prepare を呼び出して新しいプリペアドステートメントを作成します。

問題は、sql.go 内の Stmt.connStmt() メソッドのループ処理にありました。このメソッドは、s.css リストをイテレートし、各 connStmt エントリ(v)に対して、その接続(v.ci)が現在利用可能かどうかを s.db.connIfFree() で確認していました。しかし、バグのあるコードでは、ループ変数 v ではなく、ループの外で宣言された別の変数 cs の接続(cs.ci)を参照していました。

// 修正前:
for _, v := range s.css {
    // ...
    if _, match = s.db.connIfFree(cs.ci); match { // ここが問題
        cs = v
        break
    }
}

この cs 変数は、ループの最初のイテレーションで s.css の最初の要素に初期化されるか、あるいは以前の呼び出しで設定された古い値を持っている可能性がありました。結果として、ループの各イテレーションで、現在処理している v の接続ではなく、常に同じ(あるいは古い)cs の接続が connIfFree に渡されていました。

これにより、s.db.connIfFree() は常に同じ接続の状態をチェックし、たとえ v.ci が利用可能であっても、cs.ci が利用可能でなければ matchfalse になり、既存のプリペアドステートメントが再利用されないという事態が発生しました。その結果、Stmt は既存のプリペアドステートメントを再利用する代わりに、新しい接続を取得して再度 Prepare を呼び出すことになり、これが「double-Prepare」の原因となっていました。

修正は、このタイポを v.ci に変更することで、ループの現在のイテレーションで処理されている connStmt の接続が正しくチェックされるようにしました。

テストコードの変更は、このバグを検出・検証するために重要です。fakedb_test.go では、fakeConn 構造体に numPrepare カウンターを追加し、Prepare メソッドが呼び出されるたびにインクリメントするようにしました。これにより、テスト中に Prepare が何回呼び出されたかを正確に追跡できるようになります。sql_test.go では、numPrepares ヘルパー関数が追加され、TestQuery テストケース内で Prepare の呼び出し回数が期待通り(1回)であることをアサートするようになりました。これにより、将来的に同様の回帰バグが発生するのを防ぐことができます。

コアとなるコードの変更箇所

src/pkg/database/sql/fakedb_test.go

--- a/src/pkg/database/sql/fakedb_test.go
+++ b/src/pkg/database/sql/fakedb_test.go
@@ -82,6 +82,7 @@ type fakeConn struct {
 	mu          sync.Mutex
 	stmtsMade   int
 	stmtsClosed int
+	numPrepare  int
 }
 
 func (c *fakeConn) incrStat(v *int) {
@@ -339,6 +340,7 @@ func (c *fakeConn) prepareInsert(stmt *fakeStmt, parts []string) (driver.Stmt, e
 }
 
 func (c *fakeConn) Prepare(query string) (driver.Stmt, error) {
+	c.numPrepare++
 	if c.db == nil {
 		panic("nil c.db; conn = " + fmt.Sprintf("%#v", c))
 	}

src/pkg/database/sql/sql.go

--- a/src/pkg/database/sql/sql.go
+++ b/src/pkg/database/sql/sql.go
@@ -700,7 +700,7 @@ func (s *Stmt) connStmt() (ci driver.Conn, releaseConn func(), si driver.Stmt, e
 	for _, v := range s.css {
 		// TODO(bradfitz): lazily clean up entries in this
 		// list with dead conns while enumerating
-		if _, match = s.db.connIfFree(cs.ci); match {
+		if _, match = s.db.connIfFree(v.ci); match {
 			cs = v
 			break
 		}

src/pkg/database/sql/sql_test.go

--- a/src/pkg/database/sql/sql_test.go
+++ b/src/pkg/database/sql/sql_test.go
@@ -47,9 +47,19 @@ func closeDB(t *testing.T, db *DB) {
 	}\n}\n \n+// numPrepares assumes that db has exactly 1 idle conn and returns\n+// its count of calls to Prepare\n+func numPrepares(t *testing.T, db *DB) int {\n+\tif n := len(db.freeConn); n != 1 {\n+\t\tt.Fatalf(\"free conns = %d; want 1\", n)\n+\t}\n+\treturn db.freeConn[0].(*fakeConn).numPrepare\n+}\n+\n func TestQuery(t *testing.T) {\n \tdb := newTestDB(t, \"people\")\n \tdefer closeDB(t, db)\n+\tprepares0 := numPrepares(t, db)\n \trows, err := db.Query(\"SELECT|people|age,name|\")\n \tif err != nil {\n \t\tt.Fatalf(\"Query: %v\", err)\n@@ -83,7 +93,10 @@ func TestQuery(t *testing.T) {\n \t// And verify that the final rows.Next() call, which hit EOF,\n \t// also closed the rows connection.\n \tif n := len(db.freeConn); n != 1 {\n-\t\tt.Errorf(\"free conns after query hitting EOF = %d; want 1\", n)\n+\t\tt.Fatalf(\"free conns after query hitting EOF = %d; want 1\", n)\n+\t}\n+\tif prepares := numPrepares(t, db) - prepares0; prepares != 1 {\n+\t\tt.Errorf(\"executed %d Prepare statements; want 1\", prepares)\n \t}\n }\n \n```

## コアとなるコードの解説

### `src/pkg/database/sql/sql.go` の変更

```go
-		if _, match = s.db.connIfFree(cs.ci); match {
+		if _, match = s.db.connIfFree(v.ci); match {

この変更がバグの核心部分です。

  • 修正前 (cs.ci): ループ変数 v の代わりに、ループの外で宣言された cs という変数の接続 (cs.ci) を参照していました。これにより、ループが s.css リストの異なる connStmt エントリをイテレートしているにもかかわらず、常に同じ(あるいは古い)接続の状態をチェックしていました。その結果、正しい接続が利用可能であっても、matchfalse になることがあり、既存のプリペアドステートメントが再利用されず、新しい Prepare コールが発行されていました。
  • 修正後 (v.ci): ループの現在のイテレーションで処理されている connStmt エントリ v の接続 (v.ci) を正しく参照するように修正されました。これにより、s.db.connIfFree() は適切な接続の状態をチェックし、利用可能な既存のプリペアドステートメントがあればそれを再利用するようになります。これにより、不要な Prepare コールが防止され、"double-Prepare" バグが解消されます。

src/pkg/database/sql/fakedb_test.go の変更

 type fakeConn struct {
 	mu          sync.Mutex
 	stmtsMade   int
 	stmtsClosed int
+	numPrepare  int // 新しく追加されたフィールド
 }

 func (c *fakeConn) Prepare(query string) (driver.Stmt, error) {
+	c.numPrepare++ // Prepareが呼ばれるたびにインクリメント
 	// ...
 }

これはテストハーネスの変更です。

  • fakeConn は、database/sql パッケージのテストで使用されるモックのデータベース接続です。
  • numPrepare フィールドを追加することで、テスト中に Prepare メソッドが何回呼び出されたかを追跡できるようになります。
  • Prepare メソッド内で c.numPrepare++ を行うことで、このカウンターが正確に更新されます。これにより、テストケースが Prepare の呼び出し回数を検証できるようになり、バグが修正されたことを確認し、将来的な回帰を防ぐための基盤が提供されます。

src/pkg/database/sql/sql_test.go の変更

+// numPrepares assumes that db has exactly 1 idle conn and returns
+// its count of calls to Prepare
+func numPrepares(t *testing.T, db *DB) int {
+	if n := len(db.freeConn); n != 1 {
+		t.Fatalf("free conns = %d; want 1", n)
+	}
+	return db.freeConn[0].(*fakeConn).numPrepare
+}

 func TestQuery(t *testing.T) {
 	db := newTestDB(t, "people")
 	defer closeDB(t, db)
+	prepares0 := numPrepares(t, db) // テスト開始前のPrepareカウントを取得
 	rows, err := db.Query("SELECT|people|age,name|")
 	// ...
 	if n := len(db.freeConn); n != 1 {
 		t.Fatalf("free conns after query hitting EOF = %d; want 1", n)
 	}
+	if prepares := numPrepares(t, db) - prepares0; prepares != 1 { // Prepareの呼び出し回数を検証
+		t.Errorf("executed %d Prepare statements; want 1", prepares)
+	}
 }

この変更は、バグ修正を検証するための新しいテストロジックを追加しています。

  • numPrepares ヘルパー関数が追加されました。この関数は、テスト用の DB オブジェクトが持つアイドル状態の接続(freeConn)から fakeConn を取得し、その numPrepare カウンターの値を返します。これにより、テストコードが Prepare の呼び出し回数を簡単に取得できるようになります。
  • TestQuery テストケース内で、クエリ実行前と実行後に numPrepares を呼び出し、その差分を計算しています。
  • if prepares := numPrepares(t, db) - prepares0; prepares != 1 のアサーションが追加されました。これは、TestQuery の実行中に Prepare メソッドが正確に1回だけ呼び出されたことを検証します。もし2回以上呼び出された場合(つまり "double-Prepare" が発生した場合)、テストは失敗し、バグが再発したことを示します。

これらのテストの追加により、タイポバグが修正されたことを確認できるだけでなく、将来的に同様の問題が導入されることを防ぐための自動化されたチェックが確立されました。

関連リンク

参考にした情報源リンク