database/sql: document expectations for named parameters

Require parameter names to not begin with a symbol.

Change-Id: I5dfe9d4e181f0daf71dad2f395aca41c68678cbe
Reviewed-on: https://go-review.googlesource.com/33493
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index 4b4dfc40d7d135aa133badc41d7fb428008185c0..ea2f377810eac95f479066cb70024f8bc28f25dc 100644 (file)
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -13,6 +13,8 @@ import (
        "reflect"
        "strconv"
        "time"
+       "unicode"
+       "unicode/utf8"
 )
 
 var errNilPtr = errors.New("destination pointer is nil") // embedded in descriptive error
@@ -24,6 +26,17 @@ func describeNamedValue(nv *driver.NamedValue) string {
        return fmt.Sprintf("with name %q", nv.Name)
 }
 
+func validateNamedValueName(name string) error {
+       if len(name) == 0 {
+               return nil
+       }
+       r, _ := utf8.DecodeRuneInString(name)
+       if unicode.IsLetter(r) {
+               return nil
+       }
+       return fmt.Errorf("name %q does not begin with a letter", name)
+}
+
 // driverArgs converts arguments from callers of Stmt.Exec and
 // Stmt.Query into driver Values.
 //
@@ -43,6 +56,9 @@ func driverArgs(ds *driverStmt, args []interface{}) ([]driver.NamedValue, error)
                        nv := &nvargs[n]
                        nv.Ordinal = n + 1
                        if np, ok := arg.(NamedArg); ok {
+                               if err := validateNamedValueName(np.Name); err != nil {
+                                       return nil, err
+                               }
                                arg = np.Value
                                nvargs[n].Name = np.Name
                        }
@@ -60,6 +76,9 @@ func driverArgs(ds *driverStmt, args []interface{}) ([]driver.NamedValue, error)
                nv := &nvargs[n]
                nv.Ordinal = n + 1
                if np, ok := arg.(NamedArg); ok {
+                       if err := validateNamedValueName(np.Name); err != nil {
+                               return nil, err
+                       }
                        arg = np.Value
                        nv.Name = np.Name
                }

diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go
index c8cbbf06961d9343d2d3dc57fd614dc10394f9f4..2e47cd9ee73b6da1343ab57b33308068dd797304 100644 (file)
--- a/src/database/sql/driver/driver.go
+++ b/src/database/sql/driver/driver.go
@@ -27,13 +27,18 @@ import (
 type Value interface{}
 
 // NamedValue holds both the value name and value.
-// The Ordinal is the position of the parameter starting from one and is always set.
-// If the Name is not empty it should be used for the parameter identifier and
-// not the ordinal position.
 type NamedValue struct {
-       Name    string
+       // If the Name is not empty it should be used for the parameter identifier and
+       // not the ordinal position.
+       //
+       // Name will not have a symbol prefix.
+       Name string
+
+       // Ordinal position of the parameter starting from one and is always set.
        Ordinal int
-       Value   Value
+
+       // Value is the parameter value.
+       Value Value
 }
 
 // Driver is the interface that must be implemented by a database

diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go
index 416b97d501024c41982218e54c3c6962dc725268..4b15f5bec7bfb273aeb8b40bcc08b10387107934 100644 (file)
--- a/src/database/sql/fakedb_test.go
+++ b/src/database/sql/fakedb_test.go
@@ -713,7 +713,7 @@ func (s *fakeStmt) execInsert(args []driver.NamedValue, doInsert bool) (driver.R
                        } else {
                                // Assign value from argument placeholder name.
                                for _, a := range args {
-                                       if a.Name == strvalue {
+                                       if a.Name == strvalue[1:] {
                                                val = a.Value
                                                break
                                        }
@@ -818,7 +818,7 @@ func (s *fakeStmt) QueryContext(ctx context.Context, args []driver.NamedValue) (
                                } else {
                                        // Assign arg value from placeholder name.
                                        for _, a := range args {
-                                               if a.Name == wcol.Placeholder {
+                                               if a.Name == wcol.Placeholder[1:] {
                                                        argValue = a.Value
                                                        break
                                                }

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index a620707b2d7dfee4692c9e56d174e756a63d979c..e11a9dadd012f84eb3756a5793ab0d62b4ee963e 100644 (file)
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -76,6 +76,8 @@ type NamedArg struct {
 
        // Name of the parameter placeholder. If empty the ordinal position in the
        // argument list will be used.
+       //
+       // Name must omit any symbol prefix.
        Name string
 
        // Value of the parameter. It may be assigned the same value types as

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 27fb765cde87d757518f7d8e2ca970d62d760bd5..02746a2e30070c03d7489d1426cf842b70c974cc 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -486,8 +486,8 @@ func TestQueryNamedArg(t *testing.T) {
        rows, err := db.Query(
                // Ensure the name and age parameters only match on placeholder name, not position.
                "SELECT|people|age,name|name=?name,age=?age",
-               Named("?age", 2),
-               Named("?name", "Bob"),
+               Named("age", 2),
+               Named("name", "Bob"),
        )
        if err != nil {
                t.Fatalf("Query: %v", err)