Scan SQL for placeholders and named parameters while skipping literals, identifiers, comments and casts.

bind.go

@@ -53,40 +53,62 @@ func BindDriver(driverName string, bindType int) {
 	binds.Store(driverName, bindType)
 }
 
-// FIXME: this should be able to be tolerant of escaped ?'s in queries without
-// losing much speed, and should be to avoid confusion.
-
 // Rebind a query from the default bindtype (QUESTION) to the target bindtype.
+// A '?' inside a string literal, a quoted identifier or a comment is left
+// alone, and '??' is the escape for a literal '?' with every bind type.
 func Rebind(bindType int, query string) string {
 	switch bindType {
 	case QUESTION, UNKNOWN:
-		return query
+		if !strings.Contains(query, "??") {
+			return query
+		}
 	}
 
 	// Add space enough for 10 params before we have to allocate
 	rqb := make([]byte, 0, len(query)+10)
 
-	var i, j int
-
-	for i = strings.Index(query, "?"); i != -1; i = strings.Index(query, "?") {
-		rqb = append(rqb, query[:i]...)
-
-		switch bindType {
-		case DOLLAR:
-			rqb = append(rqb, '$')
-		case NAMED:
-			rqb = append(rqb, ':', 'a', 'r', 'g')
-		case AT:
-			rqb = append(rqb, '@', 'p')
+	sc := sqlScanner{q: query, opt: scanOptsFor(bindType)}
+	last, j := 0, 0
+	for {
+		kind, start, end := sc.next()
+		if kind == tokEOF {
+			break
+		}
+		switch kind {
+		case tokQuestion:
+			rqb = append(rqb, query[last:start]...)
+			switch bindType {
+			case DOLLAR:
+				rqb = append(rqb, '$')
+			case NAMED:
+				rqb = append(rqb, ':', 'a', 'r', 'g')
+			case AT:
+				rqb = append(rqb, '@', 'p')
+			default:
+				rqb = append(rqb, '?')
+				last = end
+				continue
+			}
+			j++
+			rqb = strconv.AppendInt(rqb, int64(j), 10)
+			last = end
+		case tokQuestionPair:
+			rqb = append(rqb, query[last:start]...)
+			rqb = append(rqb, '?')
+			last = end
 		}
+	}
 
-		j++
-		rqb = strconv.AppendInt(rqb, int64(j), 10)
+	return string(append(rqb, query[last:]...))
+}
 
-		query = query[i+1:]
+// scanOptsFor returns the scanner options for a bind type: SQL Server's
+// [bracketed] identifiers are only recognised for AT.
+func scanOptsFor(bindType int) scanOpts {
+	if bindType == AT {
+		return optBrackets
 	}
-
-	return string(append(rqb, query...))
+	return 0
 }
 
 // Experimental implementation of Rebind which uses a bytes.Buffer.  The code is
@@ -143,7 +165,8 @@ func asSliceForIn(i interface{}) (v reflect.Value, ok bool) {
 // targeting PostgreSQL or other drivers that normally use a different
 // placeholder style. In always returns a query that still uses `?`; call
 // db.Rebind on the result before Query/Exec so the placeholders match the
-// driver.
+// driver. A `?` inside a string literal, a quoted identifier or a comment is
+// not a bindVar, and `??` is a literal `?`.
 //
 // An empty or nil slice expands to the SQL literal NULL and binds no
 // argument, so `x IN (?)` becomes `x IN (NULL)`, which matches no row on any
@@ -205,9 +228,18 @@ func In(query string, args ...interface{}) (string, []interface{}, error) {
 	var buf strings.Builder
 	buf.Grow(len(query) + len(", ?")*flatArgsCount)
 
-	var arg, offset int
-
-	for i := strings.IndexByte(query[offset:], '?'); i != -1; i = strings.IndexByte(query[offset:], '?') {
+	// walk the query with the scanner so that a '?' inside a string literal
+	// or a comment is not taken for a bindvar; '??' and ':name' pass through
+	sc := sqlScanner{q: query}
+	arg, last := 0, 0
+	for {
+		kind, start, end := sc.next()
+		if kind == tokEOF {
+			break
+		}
+		if kind != tokQuestion {
+			continue
+		}
 		if arg >= len(meta) {
 			// if an argument wasn't passed, lets return an error;  this is
 			// not actually how database/sql Exec/Query works, but since we are
@@ -221,38 +253,30 @@ func In(query string, args ...interface{}) (string, []interface{}, error) {
 
 		// an empty slice becomes the literal NULL, which binds nothing
 		if argMeta.empty {
-			buf.WriteString(query[:offset+i])
+			buf.WriteString(query[last:start])
 			buf.WriteString("NULL")
-			query = query[offset+i+1:]
-			offset = 0
+			last = end
 			continue
 		}
 
-		// not a slice, continue.
-		// our questionmark will either be written before the next expansion
-		// of a slice or after the loop when writing the rest of the query
+		// not a slice: its '?' is copied along with the surrounding text
 		if argMeta.length == 0 {
-			offset = offset + i + 1
 			newArgs = append(newArgs, argMeta.i)
 			continue
 		}
 
 		// write everything up to and including our ? character
-		buf.WriteString(query[:offset+i+1])
+		buf.WriteString(query[last:end])
 
 		for si := 1; si < argMeta.length; si++ {
 			buf.WriteString(", ?")
 		}
 
 		newArgs = appendReflectSlice(newArgs, argMeta.v, argMeta.length)
-
-		// slice the query and reset the offset. this avoids some bookkeeping for
-		// the write after the loop
-		query = query[offset+i+1:]
-		offset = 0
+		last = end
 	}
 
-	buf.WriteString(query)
+	buf.WriteString(query[last:])
 
 	if arg < len(meta) {
 		return "", nil, errors.New("number of bindVars less than number arguments")

named.go

@@ -20,7 +20,6 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-	"unicode"
 
 	"github.com/secondwindoss/sqlx/reflectx"
 )
@@ -132,7 +131,7 @@ type namedPreparer interface {
 
 func prepareNamed(p namedPreparer, query string) (*NamedStmt, error) {
 	bindType := BindType(p.DriverName())
-	q, args, err := compileNamedQuery([]byte(query), bindType)
+	q, args, err := compileNamedQuery(query, bindType)
 	if err != nil {
 		return nil, err
 	}
@@ -223,7 +222,7 @@ func bindMapArgs(names []string, arg map[string]interface{}) ([]interface{}, err
 // The rules for binding field names to parameter names follow the same
 // conventions as for StructScan, including obeying the `db` struct tags.
 func bindStruct(bindType int, query string, arg interface{}, m *reflectx.Mapper) (string, []interface{}, error) {
-	bound, names, err := compileNamedQuery([]byte(query), bindType)
+	bound, names, err := compileNamedQuery(query, bindType)
 	if err != nil {
 		return "", []interface{}{}, err
 	}
@@ -303,7 +302,7 @@ func fixBound(bound string, loop int) string {
 func bindArray(bindType int, query string, arg interface{}, m *reflectx.Mapper) (string, []interface{}, error) {
 	// do the initial binding with QUESTION;  if bindType is not question,
 	// we can rebind it at the end.
-	bound, names, err := compileNamedQuery([]byte(query), QUESTION)
+	bound, names, err := compileNamedQuery(query, QUESTION)
 	if err != nil {
 		return "", []interface{}{}, err
 	}
@@ -332,7 +331,7 @@ func bindArray(bindType int, query string, arg interface{}, m *reflectx.Mapper)
 
 // bindMap binds a named parameter query with a map of arguments.
 func bindMap(bindType int, query string, args map[string]interface{}) (string, []interface{}, error) {
-	bound, names, err := compileNamedQuery([]byte(query), bindType)
+	bound, names, err := compileNamedQuery(query, bindType)
 	if err != nil {
 		return "", []interface{}{}, err
 	}
@@ -343,96 +342,52 @@ func bindMap(bindType int, query string, args map[string]interface{}) (string, [
 
 // -- Compilation of Named Queries
 
-// Allow digits and letters in bind params;  additionally runes are
-// checked against underscores, meaning that bind params can have be
-// alphanumeric with underscores.  Mind the difference between unicode
-// digits and numbers, where '5' is a digit but '五' is not.
-var allowedBindRunes = []*unicode.RangeTable{unicode.Letter, unicode.Digit}
-
-// FIXME: this function isn't safe for unicode named params, as a failing test
-// can testify.  This is not a regression but a failure of the original code
-// as well.  It should be modified to range over runes in a string rather than
-// bytes, even though this is less convenient and slower.  Hopefully the
-// addition of the prepared NamedStmt (which will only do this once) will make
-// up for the slightly slower ad-hoc NamedExec/NamedQuery.
-
-// compile a NamedQuery into an unbound query (using the '?' bindvar) and
-// a list of names.
-func compileNamedQuery(qs []byte, bindType int) (query string, names []string, err error) {
-	names = make([]string, 0, 10)
-	rebound := make([]byte, 0, len(qs))
-
-	inName := false
-	last := len(qs) - 1
-	currentVar := 1
-	name := make([]byte, 0, 10)
-
-	for i, b := range qs {
-		// a ':' while we're in a name is an error
-		if b == ':' {
-			// if this is the second ':' in a '::' escape sequence, append a ':'
-			if inName && i > 0 && qs[i-1] == ':' {
-				rebound = append(rebound, ':')
-				inName = false
-				continue
-			} else if inName {
-				err = errors.New("unexpected `:` while reading named param at " + strconv.Itoa(i))
-				return query, names, err
-			}
-			inName = true
-			name = []byte{}
-		} else if inName && i > 0 && b == '=' && len(name) == 0 {
-			rebound = append(rebound, ':', '=')
-			inName = false
+// compileNamedQuery turns a query with :name parameters into one with the
+// bindvars of bindType and returns the parameter names in order. A name
+// starts with a letter, an underscore or a non-ASCII character and continues
+// with letters, digits, underscores, non-ASCII characters and dots. A '::'
+// cast, a ':' inside a string literal, a quoted identifier or a comment, and
+// any other ':' not followed by a name pass through verbatim. Two names with
+// nothing between them (":first:name") are an error.
+func compileNamedQuery(query string, bindType int) (string, []string, error) {
+	names := make([]string, 0, 10)
+	rebound := make([]byte, 0, len(query)+10)
+
+	sc := sqlScanner{q: query, opt: scanOptsFor(bindType)}
+	last, prevEnd, currentVar := 0, -1, 1
+	for {
+		kind, start, end := sc.next()
+		if kind == tokEOF {
+			break
+		}
+		if kind != tokName {
 			continue
-			// if we're in a name, and this is an allowed character, continue
-		} else if inName && (unicode.IsOneOf(allowedBindRunes, rune(b)) || b == '_' || b == '.') && i != last {
-			// append the byte to the name if we are in a name and not on the last byte
-			name = append(name, b)
-			// if we're in a name and it's not an allowed character, the name is done
-		} else if inName {
-			inName = false
-			// if this is the final byte of the string and it is part of the name, then
-			// make sure to add it to the name
-			if i == last && unicode.IsOneOf(allowedBindRunes, rune(b)) {
-				name = append(name, b)
-			}
-			// add the string representation to the names list
-			names = append(names, string(name))
-			// add a proper bindvar for the bindType
-			switch bindType {
-			// oracle only supports named type bind vars even for positional
-			case NAMED:
-				rebound = append(rebound, ':')
-				rebound = append(rebound, name...)
-			case QUESTION, UNKNOWN:
-				rebound = append(rebound, '?')
-			case DOLLAR:
-				rebound = append(rebound, '$')
-				for _, b := range strconv.Itoa(currentVar) {
-					rebound = append(rebound, byte(b))
-				}
-				currentVar++
-			case AT:
-				rebound = append(rebound, '@', 'p')
-				for _, b := range strconv.Itoa(currentVar) {
-					rebound = append(rebound, byte(b))
-				}
-				currentVar++
-			}
-			// add this byte to string unless it was not part of the name
-			if i != last {
-				rebound = append(rebound, b)
-			} else if !unicode.IsOneOf(allowedBindRunes, rune(b)) {
-				rebound = append(rebound, b)
-			}
-		} else {
-			// this is a normal byte and should just go onto the rebound query
-			rebound = append(rebound, b)
 		}
+		if start == prevEnd {
+			return "", nil, errors.New("unexpected `:` while reading named param at " + strconv.Itoa(start))
+		}
+		names = append(names, query[start+1:end])
+		rebound = append(rebound, query[last:start]...)
+		switch bindType {
+		// oracle only supports named type bind vars even for positional
+		case NAMED:
+			rebound = append(rebound, query[start:end]...)
+		case DOLLAR:
+			rebound = append(rebound, '$')
+			rebound = strconv.AppendInt(rebound, int64(currentVar), 10)
+			currentVar++
+		case AT:
+			rebound = append(rebound, '@', 'p')
+			rebound = strconv.AppendInt(rebound, int64(currentVar), 10)
+			currentVar++
+		default: // QUESTION, UNKNOWN
+			rebound = append(rebound, '?')
+		}
+		last, prevEnd = end, end
 	}
+	rebound = append(rebound, query[last:]...)
 
-	return string(rebound), names, err
+	return string(rebound), names, nil
 }
 
 // BindNamed binds a struct or a map to a query with named parameters.

named_cast_test.go

@@ -0,0 +1,38 @@
+package sqlx
+
+import "testing"
+
+// A Postgres cast attached to a named parameter binds the parameter and keeps
+// the cast (upstream #983); a cast elsewhere in the query is left alone
+// (upstream #428, #650, #956). Other databases have no :: operator.
+func TestNamedPostgresTypeCast(t *testing.T) {
+	RunWithSchema(defaultSchema, t, func(db *DB, t *testing.T, now string) {
+		if db.DriverName() != "postgres" {
+			return // the :: cast is Postgres syntax
+		}
+		loadDefaultFixture(db, t)
+
+		var got struct {
+			Code  string `db:"code"`
+			Day   string `db:"day"`
+			Count int    `db:"count"`
+		}
+		rows, err := db.NamedQuery(`
+			SELECT :tel::text AS code, (now() AT TIME ZONE 'utc')::date::text AS day, count(*)::int AS count
+			FROM place WHERE telcode = :tel::int`,
+			map[string]interface{}{"tel": 852})
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer rows.Close()
+		if !rows.Next() {
+			t.Fatal("expected one row")
+		}
+		if err := rows.StructScan(&got); err != nil {
+			t.Fatal(err)
+		}
+		if got.Code != "852" || got.Count != 1 || len(got.Day) != 10 {
+			t.Errorf("got %+v", got)
+		}
+	})
+}

named_context.go

@@ -17,7 +17,7 @@ type namedPreparerContext interface {
 
 func prepareNamedContext(ctx context.Context, p namedPreparerContext, query string) (*NamedStmt, error) {
 	bindType := BindType(p.DriverName())
-	q, args, err := compileNamedQuery([]byte(query), bindType)
+	q, args, err := compileNamedQuery(query, bindType)
 	if err != nil {
 		return nil, err
 	}