Change prefixdb semantics (letsencrypt#2674)

Roland Bracewell Shoemaker · jsha · commit b38077e02eac · 2017-04-12T21:57:58.000-07:00
Instead of executing the prefix for every statement only do it when creating the connection. Leaves most of the existing naming conventions alone but updates the relevant comments to reflect setting variables is now connection level instead of statement. Fixes letsencrypt#2673.
diff --git a/prefixdb/db.go b/prefixdb/db.go
@@ -3,8 +3,8 @@ package prefixdb
 import "database/sql/driver"
 
 // New clones a database driver to create a new driver with the property that
-// every statement executed will have the given prefix prepended.
-// This is useful, for instance, to set statement-level variables like
+// every connection created will have a query executed before it is used.
+// This is useful, for instance, to set connection-level variables like
 // max_statement_time and long_query_time.
 func New(prefix string, underlying driver.Driver) driver.Driver {
 	return &prefixedDB{
@@ -23,25 +23,13 @@ func (p *prefixedDB) Open(name string) (driver.Conn, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &prefixedConn{
-		prefix: p.prefix,
-		conn:   conn,
-	}, nil
-}
-
-type prefixedConn struct {
-	prefix string
-	conn   driver.Conn
-}
-
-func (c *prefixedConn) Prepare(query string) (driver.Stmt, error) {
-	return c.conn.Prepare(c.prefix + " " + query)
-}
-
-func (c *prefixedConn) Close() error {
-	return c.conn.Close()
-}
-
-func (c *prefixedConn) Begin() (driver.Tx, error) {
-	return c.conn.Begin()
+	stmt, err := conn.Prepare(p.prefix)
+	if err != nil {
+		return nil, err
+	}
+	_, err = stmt.Exec(nil)
+	if err != nil {
+		return nil, err
+	}
+	return conn, nil
 }
diff --git a/prefixdb/db_test.go b/prefixdb/db_test.go
@@ -12,7 +12,7 @@ import (
 )
 
 func TestPrefixing(t *testing.T) {
-	sql.Register("prefixedmysql", New("SET STATEMENT max_statement_time=0.1 FOR", mysql.MySQLDriver{}))
+	sql.Register("prefixedmysql", New("SET SESSION max_statement_time=0.1", mysql.MySQLDriver{}))
 	db, err := sql.Open("prefixedmysql", vars.DBConnSA)
 	if err != nil {
 		log.Fatal(err)
diff --git a/sa/database.go b/sa/database.go
@@ -59,8 +59,8 @@ func NewDbMapFromConfig(config *mysql.Config, maxOpenConns int) (*gorp.DbMap, er
 	adjustMySQLConfig(config)
 
 	// We always want strict mode. Rather than leaving this up to DB config, we
-	// prefix each statement with it.
-	prefix := "SET STATEMENT sql_mode='STRICT_ALL_TABLES' FOR "
+	// prefix each session with it.
+	prefix := "SET SESSION sql_mode='STRICT_ALL_TABLES'"
 
 	// If a read timeout is set, we set max_statement_time to 95% of that, and
 	// long_query_time to 80% of that. That way we get logs of queries that are
@@ -72,7 +72,7 @@ func NewDbMapFromConfig(config *mysql.Config, maxOpenConns int) (*gorp.DbMap, er
 		// Note: in MySQL (which we don't use), max_statement_time is millis.
 		readTimeout := config.ReadTimeout.Seconds()
 		prefix = fmt.Sprintf(
-			"SET STATEMENT max_statement_time=%g, long_query_time=%g, sql_mode='STRICT_ALL_TABLES' FOR ",
+			"SET SESSION max_statement_time=%g, long_query_time=%g, sql_mode='STRICT_ALL_TABLES'",
 			readTimeout*0.95, readTimeout*0.80)
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ import (`
`12`	`12`	`)`
`13`	`13`
`14`	`14`	`func TestPrefixing(t *testing.T) {`
`15`		`- sql.Register("prefixedmysql", New("SET STATEMENT max_statement_time=0.1 FOR", mysql.MySQLDriver{}))`
	`15`	`+ sql.Register("prefixedmysql", New("SET SESSION max_statement_time=0.1", mysql.MySQLDriver{}))`
`16`	`16`	`db, err := sql.Open("prefixedmysql", vars.DBConnSA)`
`17`	`17`	`if err != nil {`
`18`	`18`	`log.Fatal(err)`