[インデックス 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
オブジェクト: プリペアドステートメントを表します。Query
とExec
: クエリの実行と結果の取得を行います。
プリペアドステートメント (Prepared Statement)
プリペアドステートメントは、SQLクエリをデータベースサーバーに事前に準備(コンパイル)させておく機能です。これにより、以下のような利点があります。
- パフォーマンスの向上: 同じ構造のクエリを複数回実行する場合、毎回クエリを解析・コンパイルするオーバーヘッドがなくなります。
- 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
が利用可能でなければ match
が false
になり、既存のプリペアドステートメントが再利用されないという事態が発生しました。その結果、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
エントリをイテレートしているにもかかわらず、常に同じ(あるいは古い)接続の状態をチェックしていました。その結果、正しい接続が利用可能であっても、match
がfalse
になることがあり、既存のプリペアドステートメントが再利用されず、新しい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" が発生した場合)、テストは失敗し、バグが再発したことを示します。
これらのテストの追加により、タイポバグが修正されたことを確認できるだけでなく、将来的に同様の問題が導入されることを防ぐための自動化されたチェックが確立されました。
関連リンク
- Go Code Review: https://golang.org/cl/5754057
参考にした情報源リンク
- Go言語
database/sql
パッケージのドキュメント: https://pkg.go.dev/database/sql - Go言語の
database/sql
パッケージの内部動作に関する記事 (一般的な情報源):- https://go.dev/doc/database/
- https://www.alexedwards.net/blog/go-database-sql
- https://www.calhoun.io/connecting-to-a-postgresql-database-with-go/ (PostgreSQLドライバーの文脈で
database/sql
を使用する例)
- プリペアドステートメントに関する一般的な情報: