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/driver: DefaultParameterConverter breaks driver expectations when decimalDecompose is implemented #34315

Open
richjddavis opened this issue Sep 16, 2019 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@richjddavis
Copy link

richjddavis commented Sep 16, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/richjddavis/Library/Caches/go-build"
GOENV="/Users/richjddavis/Library/Application Support/go/env"
GOEXE=""
GOFLAGS="-tags=noaws"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/richjddavis/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bq/603y7lh10433_w66hbr38t480000gn/T/go-build565497442=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Provided a "github.com/shopspring/decimal".Decimal as a query parameter, using the github.com/lib/pq driver.

You'll need a PostgreSQL server running somewhere so that sql.Open can do its thing and get to the problematic point. I'm running a 10.10 server locally, but it shouldn't matter, so long as it's able to be connected to.

package main

import (
	"database/sql"
	"fmt"
	"os"

	_ "github.com/lib/pq"
	"github.com/shopspring/decimal"
)

func main() {
	// Should `POSTGRES_CONNSTR` be not set, it'll check for a locally running instance on the default port of 5432
	db, err := sql.Open("postgres", os.Getenv("POSTGRES_CONNSTR"))
	if err != nil {
		panic(err)
	}

	in := decimal.New(101, -1)
	var out decimal.Decimal

	row := db.QueryRow("select $1", in)
	if err := row.Scan(&out); err != nil {
		panic(err)
	}

	fmt.Println(in, out, in.Equal(out))
}

What did you expect to see?

POSTGRES_CONNSTR='sslmode=disable' go run main.go
10.1 10.1 true

What did you see instead?

POSTGRES_CONNSTR='sslmode=disable' go run main.go
panic: pq: encode: unknown type for decimal.Decimal

goroutine 1 [running]:
main.main()
        ~/go/src/github.com/richjddavis/go-1-13-pq-decimal/main.go:23 +0x35f
exit status 2

This seems to happen because due to the intersection of a few different behaviours:

This comment in driver.defaultConverter.ConvertValue() seems to suggest that if a type implements Valuer (which "github.com/shopspring/decimal".Decimal does), then that implementation should be used over decimalDecompose. However, IsValue, which has a positive match for decimalDecompose, is checked prior to this, and early exits without any conversion. If I remove the case for decimalDecompose in driver.IsValue(), the sample program runs as expected (and as it did in Go 1.12).

I'm not really sure which layer is "at fault" here, but in response to someone else raising this in in shopspring/decimal#141 (comment), @kardianos had requested that an issue be created here and that they be mentioned.

@kardianos kardianos self-assigned this Sep 16, 2019
@kardianos
Copy link
Contributor

kardianos commented Sep 16, 2019

Thanks for writing up a more complete detailed issue and for posting it on this issue list.

@adriendomoison
Copy link

adriendomoison commented Sep 17, 2019

Any workaround or recommendation for this until this is fixed? I just deleted all my go.mod, kept only one at the root, and updated to go 1.3 - which started this error. Should I downgrade for now?

@richjddavis
Copy link
Author

richjddavis commented Sep 17, 2019

@adriendomoison I've worked around this by removing (really, putting behind a go114 build flag) the implementations of Compose() and Decompose() in the decimal library. Looks like you're using Go modules, so you could probably also pin to a version of your library from before those functions were implemented?

@risentveber
Copy link

risentveber commented Oct 7, 2019

Seems that by now the only way is to use previous decimal version, so add this to go.mod

github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24

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

5 participants