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.

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

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

Open
flimzy opened this issue Jul 8, 2024 · 4 comments
Open

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

flimzy opened this issue Jul 8, 2024 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@flimzy
Copy link
Contributor

flimzy commented Jul 8, 2024

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
Copy link
Contributor

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

@thanm
Copy link
Contributor

thanm commented Jul 8, 2024

@bradfitz @kardianos per owners.

@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 8, 2024
@thanm thanm added this to the Backlog milestone Jul 8, 2024
@thanm
Copy link
Contributor

thanm commented Jul 8, 2024

Thanks for sending a CL --

@flimzy flimzy changed the title databe/sql: default argument conversion is bypassed for ColumnConverters that return -1 for NumInputs() databe/sql: ColumnConverter is bypassed when NumInout returns -1 Jul 8, 2024
@flimzy flimzy changed the title databe/sql: ColumnConverter is bypassed when NumInout returns -1 databe/sql: ColumnConverter is bypassed when NumInput returns -1 Jul 8, 2024
@seankhliao seankhliao changed the title databe/sql: ColumnConverter is bypassed when NumInput returns -1 database/sql: ColumnConverter is bypassed when NumInput returns -1 Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants