Add QueryerContext interface

[surki]

If we don't support QueryerContext, the db.Query() call will always do
"prepare" statement

Solves issue #22

[coveralls]

Coverage decreased (-1.7%) to 65.174% when pulling 98e9bd8 on surki:master into 7408a7f on gchaincl:master.

[coveralls]

Coverage decreased (-1.7%) to 65.174% when pulling 98e9bd8 on surki:master into 7408a7f on gchaincl:master.

[coveralls]

Coverage decreased (-1.7%) to 65.174% when pulling 98e9bd8 on surki:master into 7408a7f on gchaincl:master.

[coveralls]

Coverage increased (+7.7%) to 74.545% when pulling 5b518ef on surki:master into 7408a7f on gchaincl:master.

[qustavo]

Thanks for doing this @surki . Is there a way to integrate benchmark this?

[surki]

@gchaincl Do you want me to benchmark with / without this change?

Here is what I see

sqlhook master (without this changes):

go test -bench=. -benchmem
goos: linux
goarch: amd64
BenchmarkSQLite3/Without_Hooks-8                  200000              8856 ns/op             752 B/op         14 allocs/op
BenchmarkSQLite3/With_Hooks-8                     200000             10370 ns/op             864 B/op         16 allocs/op
BenchmarkMySQL/Without_Hooks-8                     20000             68749 ns/op             629 B/op         11 allocs/op
BenchmarkMySQL/With_Hooks-8                        10000            116692 ns/op             774 B/op         14 allocs/op
BenchmarkPostgres/Without_Hooks-8                  20000             68941 ns/op             800 B/op         17 allocs/op
BenchmarkPostgres/With_Hooks-8                     10000            156187 ns/op            1273 B/op         27 allocs/op
PASS
ok      _/home/suresh/src/sqlhooks      11.130s

With this change:

$ go test -bench=. -benchmem
goos: linux
goarch: amd64
BenchmarkSQLite3/Without_Hooks-8                  200000              9237 ns/op             752 B/op         14 allocs/op
BenchmarkSQLite3/With_Hooks-8                     200000              9466 ns/op             752 B/op         14 allocs/op
BenchmarkMySQL/Without_Hooks-8                     20000             68764 ns/op             629 B/op         11 allocs/op
BenchmarkMySQL/With_Hooks-8                        30000             59447 ns/op             629 B/op         11 allocs/op
BenchmarkPostgres/Without_Hooks-8                  20000             69172 ns/op             800 B/op         17 allocs/op
BenchmarkPostgres/With_Hooks-8                     20000             66238 ns/op             800 B/op         17 allocs/op
PASS
ok      _/home/suresh/src/sqlhooks      12.652s

Weirdly WithHooks is faster now, need to check how it got faster

[qustavo]

Interesting! I'd check whether mysql or psql implement *ExecerContext and *QueryerContext

[surki]

They do implement, verified it.
Did a quick test with dlv and pprof, both WithHooks and WithoutHooks take same code path.
See below callstack for db.Query() call, all the way to mysqlConn.query.

Without hook                                                                   With hook

github.com/go-sql-driver/mysql.(*mysqlConn).query                              github.com/go-sql-driver/mysql.(*mysqlConn).query        
github.com/go-sql-driver/mysql.(*mysqlConn).QueryContext                       github.com/go-sql-driver/mysql.(*mysqlConn).QueryContext
                                                                               sqlhooks.(*QueryerContext).queryContext
                                                                               sqlhooks.(*QueryerContext).QueryContext
database/sql.ctxDriverQuery                                                    database/sql.ctxDriverQuery            
database/sql.(*DB).queryDC.func1                                               database/sql.(*DB).queryDC.func1       
database/sql.withLock                                                          database/sql.withLock                  
database/sql.(*DB).queryDC                                                     database/sql.(*DB).queryDC             
database/sql.(*DB).query                                                       database/sql.(*DB).query               
database/sql.(*DB).QueryContext                                                database/sql.(*DB).QueryContext        
database/sql.(*DB).Query                                                       database/sql.(*DB).Query               
main.benchmark                                                                 main.benchmark                         
main.main                                                                      main.main                              
runtime.main                                                                   runtime.main                           
runtime.goexit                                                                 runtime.goexit                         

I am in middle of something, I will get to this next week for further check

[qustavo]

Wow, that's impressive! Thank you so much for doing it.
I am looking forward to seeing what you find out.

[surki]

SessionResetter interface was the difference (MySQL driver implements it, so it was doing extra work that SQLHook wasn't doing, so SQLHooks was faster ...).

I have implemented that, and as well as added unit tests for interface implementation.

Numbers now (overhead is negligible):

$ go test -bench=.
goos: linux
goarch: amd64
BenchmarkSQLite3/Without_Hooks-8                  200000              9137 ns/op
BenchmarkSQLite3/With_Hooks-8                     200000              9361 ns/op
BenchmarkMySQL/Without_Hooks-8                     20000             69912 ns/op
BenchmarkMySQL/With_Hooks-8                        20000             70635 ns/op
BenchmarkPostgres/Without_Hooks-8                  20000             67732 ns/op
BenchmarkPostgres/With_Hooks-8                     20000             68989 ns/op
PASS
ok      _/home/suresh/src/sqlhooks      12.343s

[qustavo]

That makes much more sense, this is a great piece of work!

[qustavo]

@keegancsmith I'd love to have your input on this one

[keegancsmith]

LGTM

[surki]

hmm, so looks like driver.SessionResetter is go 1.10+. That makes SQLHook require go 1.10+.
Is it okay? If not, we can disable SessionResetter changes for now (but that will change behavior of underlying driver when going through SQLHooks).

Or we could conditional compile it only for go1.10+: surki@bcd1551

[qustavo]

surki@bcd1551 looks like the right approach to me

[qustavo]

Please update with the build tag changes and merge

[qustavo]

@surki https://github.com/gchaincl/sqlhooks/invitations

[surki]

Okay, thanks, we probably should update the benchmark numbers in README

[qustavo]

Good point

…

On Thu, Feb 28, 2019, 08:37 Suresh Kumar ***@***.***> wrote: Okay, thanks, we probably should update the benchmark numbers in README — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#23 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABOoeY6H_MqsfyxOSWMXQaz_w9QQQtZVks5vR4dEgaJpZM4ZnpBb> .