Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored Rebind() so that it respects escaped ?'s #768

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactored Rebind() so that it respects escaped ?'s #768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 70 additions & 3 deletions bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,83 @@ 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.
// Rebind will not replace properly escaped question marks as part of a valid
// string constant i.e. surrounded by single or double quotes. If a double or
// single quote is inside a string constant and is properly escaped it is not
// interpreted as the end of the string. You can safely escape a quotation
// mark by preceding it with a c style backslash or with another quotation mark.
func Rebind(bindType int, query string) string {
switch bindType {
case QUESTION, UNKNOWN:
return query
}

// Make space enough for 10 named params before we have to allocate
rqb := make([]byte, len(query)+50)

inDouble := false
inSingle := false
var lastChar uint8 = 0

var k = 0
var j = 1

for i := 0; i < len(query); i++ {
c := query[i]

if c == '?' && !inSingle && !inDouble {
switch bindType {
case DOLLAR:
rqb[k] = '$'
k++
case NAMED:
rqb[k] = ':'
rqb[k+1] = 'a'
rqb[k+2] = 'r'
rqb[k+3] = 'g'
k = k + 4
case AT:
rqb[k] = '@'
rqb[k+1] = 'p'
k = k + 2
}

// Append the arg number
num := strconv.Itoa(j)
for _, n := range num {
rqb[k] = byte(n)
k++
}
j++
continue
}
rqb[k] = c

if c == '"' && !inSingle && lastChar != '\\' {
inDouble = !inDouble
}

if c == '\'' && !inDouble && lastChar != '\\' {
inSingle = !inSingle
}

k++
lastChar = c
}

// The length of rqb is likely longer than the slice we wrote to, so we only
// return the written portion.
return string(rqb[:k])
}

// The original Rebind function kept for benchmarking purposes.
func rebindIndex(bindType int, query string) string {
switch bindType {
case QUESTION, UNKNOWN:
return query
}

// Add space enough for 10 params before we have to allocate
rqb := make([]byte, 0, len(query)+10)

Expand Down
114 changes: 114 additions & 0 deletions bind_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,117 @@ func BenchmarkBindSpeed(b *testing.B) {

})
}

func TestNewRebind(t *testing.T) {
var tests = []struct {
name string
query string
question string
dollar string
named string
at string
}{
{
name: "q1",
query: `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
question: `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
dollar: `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`,
named: `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (:arg1, :arg2, :arg3, :arg4, :arg5, :arg6, :arg7, :arg8, :arg9, :arg10)`,
at: `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10)`,
},
{
name: "q2",
query: `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`,
question: `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`,
dollar: `INSERT INTO foo (a, b, c) VALUES ($1, $2, "foo"), ("Hi", $3, $4)`,
named: `INSERT INTO foo (a, b, c) VALUES (:arg1, :arg2, "foo"), ("Hi", :arg3, :arg4)`,
at: `INSERT INTO foo (a, b, c) VALUES (@p1, @p2, "foo"), ("Hi", @p3, @p4)`,
},
{
name: "q3: question escaped",
query: `SELECT * FROM test_table where id = ? AND name = 'four?'`,
question: `SELECT * FROM test_table where id = ? AND name = 'four?'`,
dollar: `SELECT * FROM test_table where id = $1 AND name = 'four?'`,
named: `SELECT * FROM test_table where id = :arg1 AND name = 'four?'`,
at: `SELECT * FROM test_table where id = @p1 AND name = 'four?'`,
},
{
name: "q4: escaped single quote and escaped question mark",
query: `INSERT INTO test_table (name, country) VALUES ('Maybe O''Reilly?', 'US') WHERE id = ?`,
question: `INSERT INTO test_table (name, country) VALUES ('Maybe O''Reilly?', 'US') WHERE id = ?`,
dollar: `INSERT INTO test_table (name, country) VALUES ('Maybe O''Reilly?', 'US') WHERE id = $1`,
named: `INSERT INTO test_table (name, country) VALUES ('Maybe O''Reilly?', 'US') WHERE id = :arg1`,
at: `INSERT INTO test_table (name, country) VALUES ('Maybe O''Reilly?', 'US') WHERE id = @p1`,
},
{
name: "q5: escaped double quote and escaped question mark",
query: `INSERT INTO test_table ("inches aka"" is the column name i think?") VALUES (42) WHERE id = ?`,
question: `INSERT INTO test_table ("inches aka"" is the column name i think?") VALUES (42) WHERE id = ?`,
dollar: `INSERT INTO test_table ("inches aka"" is the column name i think?") VALUES (42) WHERE id = $1`,
named: `INSERT INTO test_table ("inches aka"" is the column name i think?") VALUES (42) WHERE id = :arg1`,
at: `INSERT INTO test_table ("inches aka"" is the column name i think?") VALUES (42) WHERE id = @p1`,
},
{
name: "q6: backslash escaped quote and escaped question mark",
query: `INSERT INTO test_table (name, country) VALUES ('Maybe O\'Reilly?', 'US') WHERE id = ?`,
question: `INSERT INTO test_table (name, country) VALUES ('Maybe O\'Reilly?', 'US') WHERE id = ?`,
dollar: `INSERT INTO test_table (name, country) VALUES ('Maybe O\'Reilly?', 'US') WHERE id = $1`,
named: `INSERT INTO test_table (name, country) VALUES ('Maybe O\'Reilly?', 'US') WHERE id = :arg1`,
at: `INSERT INTO test_table (name, country) VALUES ('Maybe O\'Reilly?', 'US') WHERE id = @p1`,
},
}

for _, test := range tests {
q := Rebind(QUESTION, test.query)
if q != test.question {
t.Errorf("%s failed at 'question'", test.name)
}
d := Rebind(DOLLAR, test.query)
if d != test.dollar {
t.Errorf("%s failed at 'dollar'", test.name)
}
n := Rebind(NAMED, test.query)
if n != test.named {
t.Errorf("%s failed at 'named'", test.name)
}
a := Rebind(AT, test.query)
if a != test.at {
t.Errorf("%s failed at 'at'", test.name)
}
}
}

/*
goos: linux
goarch: amd64
pkg: github.com/jmoiron/sqlx
BenchmarkRebind/index-8 2114181 598 ns/op
BenchmarkRebind/new-8 2827218 747 ns/op
BenchmarkRebind/buff-8 1000000 1678 ns/op
*/

func BenchmarkRebind(b *testing.B) {
q1 := `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`
q2 := `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`

b.Run("index", func(b *testing.B) {
for i := 0; i < b.N; i++ {
rebindIndex(DOLLAR, q1)
rebindIndex(DOLLAR, q2)
}
})

b.Run("new", func(b *testing.B) {
for i := 0; i < b.N; i++ {
Rebind(DOLLAR, q1)
Rebind(DOLLAR, q2)
}
})

b.Run("buff", func(b *testing.B) {
for i := 0; i < b.N; i++ {
rebindBuff(DOLLAR, q1)
rebindBuff(DOLLAR, q2)
}
})
}
65 changes: 0 additions & 65 deletions sqlx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1316,47 +1316,6 @@ func TestDoNotPanicOnConnect(t *testing.T) {
}
}

func TestRebind(t *testing.T) {
q1 := `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
q2 := `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`

s1 := Rebind(DOLLAR, q1)
s2 := Rebind(DOLLAR, q2)

if s1 != `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)` {
t.Errorf("q1 failed")
}

if s2 != `INSERT INTO foo (a, b, c) VALUES ($1, $2, "foo"), ("Hi", $3, $4)` {
t.Errorf("q2 failed")
}

s1 = Rebind(AT, q1)
s2 = Rebind(AT, q2)

if s1 != `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10)` {
t.Errorf("q1 failed")
}

if s2 != `INSERT INTO foo (a, b, c) VALUES (@p1, @p2, "foo"), ("Hi", @p3, @p4)` {
t.Errorf("q2 failed")
}

s1 = Rebind(NAMED, q1)
s2 = Rebind(NAMED, q2)

ex1 := `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES ` +
`(:arg1, :arg2, :arg3, :arg4, :arg5, :arg6, :arg7, :arg8, :arg9, :arg10)`
if s1 != ex1 {
t.Error("q1 failed on Named params")
}

ex2 := `INSERT INTO foo (a, b, c) VALUES (:arg1, :arg2, "foo"), ("Hi", :arg3, :arg4)`
if s2 != ex2 {
t.Error("q2 failed on Named params")
}
}

func TestBindMap(t *testing.T) {
// Test that it works..
q1 := `INSERT INTO foo (a, b, c, d) VALUES (:name, :age, :first, :last)`
Expand Down Expand Up @@ -1829,30 +1788,6 @@ func BenchmarkIn1kString(b *testing.B) {
}
}

func BenchmarkRebind(b *testing.B) {
b.StopTimer()
q1 := `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`
q2 := `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`
b.StartTimer()

for i := 0; i < b.N; i++ {
Rebind(DOLLAR, q1)
Rebind(DOLLAR, q2)
}
}

func BenchmarkRebindBuffer(b *testing.B) {
b.StopTimer()
q1 := `INSERT INTO foo (a, b, c, d, e, f, g, h, i) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`
q2 := `INSERT INTO foo (a, b, c) VALUES (?, ?, "foo"), ("Hi", ?, ?)`
b.StartTimer()

for i := 0; i < b.N; i++ {
rebindBuff(DOLLAR, q1)
rebindBuff(DOLLAR, q2)
}
}

func TestIn130Regression(t *testing.T) {
t.Run("[]interface{}{}", func(t *testing.T) {
q, args, err := In("SELECT * FROM people WHERE name IN (?)", []interface{}{[]string{"gopher"}}...)
Expand Down