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 {

Comment From: gopherbot

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

Comment From: thanm

@bradfitz @kardianos per owners.

Comment From: thanm

Thanks for sending a CL --

Comment From: gabyhelp

Related Issues and Documentation

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

Comment From: flimzy

Is there anything I can do to help move this forward? It's tagged as needs investigation, but I'm unsure what investigation is needed. If that can be clarified, I'm happy to do some legwork and report back.

Comment From: ianlancetaylor

I'm concerned that passing -1 to the ColumnConverter method could break existing drivers that don't expect that. The comment in CheckNamedValue says

    // The column converter shouldn't be called on any index
    // it isn't expecting. The final error will be thrown
    // in the argument converter loop.

Is there some reason to think that any arbitrary column converter will support -1?

Could your proxy return driver.ErrSkip when proxying ColumnConverter to a driver that doesn't implement that method?

Comment From: flimzy

I'm concerned that passing -1 to the ColumnConverter method could break existing drivers that don't expect that. The comment in CheckNamedValue says

I don't believe it will ever pass -1 to ColumnConverter. The argument passed is index, which is always >= 0, as it represents the actual index of the query parameter, irrespective of the value returned by NumInput().

Could your proxy return driver.ErrSkip when proxying ColumnConverter to a driver that doesn't implement that method?

That does not solve the problem, because when NumInput() returns -1, the ColumnConverter is never even called, so that error isn't ever received for consideration.

Comment From: ianlancetaylor

Oh, right. Thanks.

The CL has picked up a merge conflict somewhere.

Comment From: flimzy

CL updated to resolve conflicts.

Comment From: flimzy

@ianlancetaylor Any further attention this issue needs?

Comment From: flimzy

I believe all outstanding concerns have been addressed. Can we get some eyes on this, for possible merge? Not sure who the best person to tag is, now that Ian has left Google. cc/ @bradfitz @kardianos ?