golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.58k stars 17.41k forks source link

database/sql: ColumnConverter is bypassed when NumInput returns -1 #68342

Open flimzy opened 2 weeks ago

flimzy commented 2 weeks ago

Go version

go version go1.22.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/jonhall/.cache/go-build'
GOENV='/home/jonhall/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/jonhall/go/pkg/mod'
GONOPROXY='gitlab.com/FlashbackSRS/priv,gitlab.com/flimzy/hacker-portfolio'
GONOSUMDB='gitlab.com/FlashbackSRS/priv,gitlab.com/flimzy/hacker-portfolio'
GOOS='linux'
GOPATH='/home/jonhall/go'
GOPRIVATE='gitlab.com/FlashbackSRS/priv,gitlab.com/flimzy/hacker-portfolio'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/jonhall/src/bugrepro/go.mod'
GOWORK=''
CGO_CFLAGS='-w'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1095939242=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I wrote a database/sql driver that implements the optional ColumnConverter interface for prepared statements, and for which NumInputs() returns -1.

Technically, I wrote a databse/sql proxy driver, for the purpose of adding hooks to DB operations, to aide in debugging. See https://gitlab.com/flimzy/errsql

But using this library with modernc.org/sqlite is what triggered the buggy behavior. modernc.org/sqlite itself does not implement the ColumnConverter interface, but it does return -1 for NumInputs. Because the proxy driver does implement ColumnConverter (because it must implement it for all drivers it proxies, or none), it triggers the observed behavior.

What did you see happen?

The ColumnConverter method is never called, leading to the following error from sqlite:

invalid driver.Value type int

What did you expect to see?

I expected that ColumnConverter would be called.

Analysis & Possible Fix

This line appears to be the culprit:

    if c.want <= index {

c.want is set to the return value from stmt.NumInputs(), so is -1 in this case. The method containing this code is only executed when ColumnConverter is implemented, as we can see here.

I believe the solution is to change the above mentioned line to:

    if c.want > 0 && c.want <= index {
gopherbot commented 2 weeks ago

Change https://go.dev/cl/597115 mentions this issue: database/sql: Don't skip default argument conversion for ColumnConverters and unknown input count

thanm commented 2 weeks ago

@bradfitz @kardianos per owners.

thanm commented 2 weeks ago

Thanks for sending a CL --

gabyhelp commented 2 weeks ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)