-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
rebind safety #775
base: master
Are you sure you want to change the base?
rebind safety #775
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,27 @@ | ||
github.com/alvaroloes/enumer v1.1.2/go.mod h1:FxrjvuXoDAx9isTJrv4c+T410zFi0DtXIT0m65DJ+Wo= | ||
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= | ||
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= | ||
github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfCHuOE= | ||
github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= | ||
github.com/lib/pq v1.2.0 h1:LXpIM/LZ5xGFhOpXAQUIMM1HdyqzVYM13zNdjCEEcA0= | ||
github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= | ||
github.com/mattn/go-sqlite3 v1.14.6 h1:dNPt6NO46WmLVt2DLNpwczCmdV5boIZ6g/tlDrlRUbg= | ||
github.com/mattn/go-sqlite3 v1.14.6/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU= | ||
github.com/muir/sqltoken v0.0.4 h1:SioNnG90ZYXmlfnPaUxUdNC1dFkhKL64pDeS+wXZ8k8= | ||
github.com/muir/sqltoken v0.0.4/go.mod h1:6hPsZxszMpYyNf12og4f4VShFo/Qipz6Of0cn5KGAAU= | ||
github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1/go.mod h1:eD5JxqMiuNYyFNmyY9rkJ/slN8y59oEu4Ei7F8OoKWQ= | ||
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= | ||
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= | ||
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= | ||
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= | ||
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= | ||
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= | ||
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= | ||
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= | ||
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= | ||
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= | ||
golang.org/x/tools v0.0.0-20190524210228-3d17549cdc6b/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= | ||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= | ||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= | ||
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= | ||
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,20 +8,21 @@ import ( | |
|
||
func TestCompileQuery(t *testing.T) { | ||
table := []struct { | ||
d string | ||
Q, R, D, T, N string | ||
V []string | ||
}{ | ||
// basic test for named parameters, invalid char ',' terminating | ||
{ | ||
d: "basic test for named parameters, invalid char ',' terminating", | ||
Q: `INSERT INTO foo (a,b,c,d) VALUES (:name, :age, :first, :last)`, | ||
R: `INSERT INTO foo (a,b,c,d) VALUES (?, ?, ?, ?)`, | ||
D: `INSERT INTO foo (a,b,c,d) VALUES ($1, $2, $3, $4)`, | ||
T: `INSERT INTO foo (a,b,c,d) VALUES (@p1, @p2, @p3, @p4)`, | ||
N: `INSERT INTO foo (a,b,c,d) VALUES (:name, :age, :first, :last)`, | ||
V: []string{"name", "age", "first", "last"}, | ||
}, | ||
// This query tests a named parameter ending the string as well as numbers | ||
{ | ||
d: "This query tests a named parameter ending the string as well as numbers", | ||
Q: `SELECT * FROM a WHERE first_name=:name1 AND last_name=:name2`, | ||
R: `SELECT * FROM a WHERE first_name=? AND last_name=?`, | ||
D: `SELECT * FROM a WHERE first_name=$1 AND last_name=$2`, | ||
|
@@ -30,15 +31,15 @@ func TestCompileQuery(t *testing.T) { | |
V: []string{"name1", "name2"}, | ||
}, | ||
{ | ||
Q: `SELECT "::foo" FROM a WHERE first_name=:name1 AND last_name=:name2`, | ||
Q: `SELECT ":foo" FROM a WHERE first_name=:name1 AND last_name=:name2`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume idea here is/was that As an aside, this library is "sporadically maintained", at best, so I'm not expecting a merge or feedback from the maintainer(s) any time soon, so I wouldn't rush to fix this. I'm (probably) going to use in my own library/fork I'm working on though. Just wanted to point this out. Also might be nice to replace those four calls to testify in your library? Don't want to start a discussion about testify vs. stdlib tests, but smaller dependency trees are always nice, and with just four calls it's not that much benefit 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I called this out in my PR description. Since this is not backwards compatible it requires a major version change. The current behavior isn't documented, but people may still be relying on it. If getting rid of the calls to testify would mean this gets merged, I would certainly do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh sorry, I missed that 😅
I have no say in the matter at all; just something I noticed. |
||
R: `SELECT ":foo" FROM a WHERE first_name=? AND last_name=?`, | ||
D: `SELECT ":foo" FROM a WHERE first_name=$1 AND last_name=$2`, | ||
T: `SELECT ":foo" FROM a WHERE first_name=@p1 AND last_name=@p2`, | ||
N: `SELECT ":foo" FROM a WHERE first_name=:name1 AND last_name=:name2`, | ||
V: []string{"name1", "name2"}, | ||
}, | ||
{ | ||
Q: `SELECT 'a::b::c' || first_name, '::::ABC::_::' FROM person WHERE first_name=:first_name AND last_name=:last_name`, | ||
Q: `SELECT 'a:b:c' || first_name, '::ABC:_:' FROM person WHERE first_name=:first_name AND last_name=:last_name`, | ||
R: `SELECT 'a:b:c' || first_name, '::ABC:_:' FROM person WHERE first_name=? AND last_name=?`, | ||
D: `SELECT 'a:b:c' || first_name, '::ABC:_:' FROM person WHERE first_name=$1 AND last_name=$2`, | ||
T: `SELECT 'a:b:c' || first_name, '::ABC:_:' FROM person WHERE first_name=@p1 AND last_name=@p2`, | ||
|
@@ -53,49 +54,66 @@ func TestCompileQuery(t *testing.T) { | |
T: `SELECT @name := "name", @p1, @p2, @p3`, | ||
V: []string{"age", "first", "last"}, | ||
}, | ||
/* This unicode awareness test sadly fails, because of our byte-wise worldview. | ||
* We could certainly iterate by Rune instead, though it's a great deal slower, | ||
* it's probably the RightWay(tm) | ||
{ | ||
Q: `INSERT INTO foo (a,b,c,d) VALUES (:あ, :b, :キコ, :名前)`, | ||
R: `INSERT INTO foo (a,b,c,d) VALUES (?, ?, ?, ?)`, | ||
D: `INSERT INTO foo (a,b,c,d) VALUES ($1, $2, $3, $4)`, | ||
N: []string{"name", "age", "first", "last"}, | ||
N: `INSERT INTO foo (a,b,c,d) VALUES (:あ, :b, :キコ, :名前)`, | ||
T: `INSERT INTO foo (a,b,c,d) VALUES (@p1, @p2, @p3, @p4)`, | ||
V: []string{"あ", "b", "キコ", "名前"}, | ||
}, | ||
{ | ||
Q: `SELECT id, added_at::date FROM person WHERE first_name=:first_name AND last_name=:last_name`, | ||
R: `SELECT id, added_at::date FROM person WHERE first_name=? AND last_name=?`, | ||
D: `SELECT id, added_at::date FROM person WHERE first_name=$1 AND last_name=$2`, | ||
T: `SELECT id, added_at::date FROM person WHERE first_name=@p1 AND last_name=@p2`, | ||
N: `SELECT id, added_at::date FROM person WHERE first_name=:first_name AND last_name=:last_name`, | ||
V: []string{"first_name", "last_name"}, | ||
}, | ||
*/ | ||
} | ||
|
||
for _, test := range table { | ||
qr, names, err := compileNamedQuery([]byte(test.Q), QUESTION) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
if qr != test.R { | ||
t.Errorf("expected %s, got %s", test.R, qr) | ||
test := test | ||
n := test.d | ||
if n == "" { | ||
n = test.Q | ||
} | ||
if len(names) != len(test.V) { | ||
t.Errorf("expected %#v, got %#v", test.V, names) | ||
} else { | ||
for i, name := range names { | ||
if name != test.V[i] { | ||
t.Errorf("expected %dth name to be %s, got %s", i+1, test.V[i], name) | ||
t.Run(n, func(t *testing.T) { | ||
if test.d != "" { | ||
t.Log(test.d) | ||
} | ||
t.Log(test.Q) | ||
qr, names, err := compileNamedQuery([]byte(test.Q), QUESTION) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
if qr != test.R { | ||
t.Errorf("R: expected %s, got(R) %s", test.R, qr) | ||
} | ||
if len(names) != len(test.V) { | ||
t.Errorf("V: expected %#v, got(V) %#v", test.V, names) | ||
} else { | ||
for i, name := range names { | ||
if name != test.V[i] { | ||
t.Errorf("expected %dth name to be %s, got(V) %s", i+1, test.V[i], name) | ||
} | ||
} | ||
} | ||
} | ||
qd, _, _ := compileNamedQuery([]byte(test.Q), DOLLAR) | ||
if qd != test.D { | ||
t.Errorf("\nexpected: `%s`\ngot: `%s`", test.D, qd) | ||
} | ||
qd, _, _ := compileNamedQuery([]byte(test.Q), DOLLAR) | ||
if qd != test.D { | ||
t.Errorf("\nexpected: `%s`\ngot(D): `%s`", test.D, qd) | ||
} | ||
|
||
qt, _, _ := compileNamedQuery([]byte(test.Q), AT) | ||
if qt != test.T { | ||
t.Errorf("\nexpected: `%s`\ngot: `%s`", test.T, qt) | ||
} | ||
qt, _, _ := compileNamedQuery([]byte(test.Q), AT) | ||
if qt != test.T { | ||
t.Errorf("\nexpected: `%s`\ngot(T): `%s`", test.T, qt) | ||
} | ||
|
||
qq, _, _ := compileNamedQuery([]byte(test.Q), NAMED) | ||
if qq != test.N { | ||
t.Errorf("\nexpected: `%s`\ngot: `%s`\n(len: %d vs %d)", test.N, qq, len(test.N), len(qq)) | ||
} | ||
qq, _, _ := compileNamedQuery([]byte(test.Q), NAMED) | ||
if qq != test.N { | ||
t.Errorf("\nexpected: `%s`\ngot(N): `%s`\n(len: %d vs %d)", test.N, qq, len(test.N), len(qq)) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
|
@@ -123,7 +141,7 @@ func (t Test) Errorf(err error, format string, args ...interface{}) { | |
|
||
func TestEscapedColons(t *testing.T) { | ||
t.Skip("not sure it is possible to support this in general case without an SQL parser") | ||
var qs = `SELECT * FROM testtable WHERE timeposted BETWEEN (now() AT TIME ZONE 'utc') AND | ||
qs := `SELECT * FROM testtable WHERE timeposted BETWEEN (now() AT TIME ZONE 'utc') AND | ||
(now() AT TIME ZONE 'utc') - interval '01:30:00') AND name = '\'this is a test\'' and id = :id` | ||
_, _, err := compileNamedQuery([]byte(qs), DOLLAR) | ||
if err != nil { | ||
|
@@ -298,7 +316,6 @@ func TestNamedQueries(t *testing.T) { | |
if p2.Email != sl.Email { | ||
t.Errorf("expected %s, got %s", sl.Email, p2.Email) | ||
} | ||
|
||
}) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was made by
gofmt
and so were a bunch of other similar changes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're using gofumpt rather than gofmt? gofmt doesn't make these kind of changes anyway.