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

Wrapping for database use #143

Open
aarondl opened this issue Oct 9, 2019 · 3 comments
Open

Wrapping for database use #143

aarondl opened this issue Oct 9, 2019 · 3 comments

Comments

@aarondl
Copy link

aarondl commented Oct 9, 2019

Hi @ericlagergren, more trouble from the sqlboiler world :)

Issue volatiletech/sqlboiler#607 details the failure to be able to insert our wrapper type into the database anymore. After reconstructing a small sample that looks like this:

	var inDec types.Decimal
	inDec.Big, _ = new(decimal.Big).SetString("1.0")

	_, err = db.Exec(`insert into objects (value) values ($1);`, &inDec)
	if err != nil {
		panic(err)
	}

	var outDec types.Decimal
	row := db.QueryRow(`select value from objects limit 1;`)
	if err != nil {
		panic(err)
	}
	err = row.Scan(&outDec)
	if err != nil {
		panic(err)
	}

	fmt.Println(outDec.String())

I've found the following results:

  • On v3.3.1 this code simply works
  • On the master branch of decimal this same code fails with an error of panic: pq: encode: unknown type for *types.Decimal (the insert error check line)

I don't pretend the blame is not on our end. As we've been marshalling with this basic string-based wrapper for some time now: https://github.com/volatiletech/sqlboiler/blob/master/types/decimal.go#L97-L152

Admittedly quite the hack, but it seems to have worked until now. I suspect the addition of the decomposer interface implementation in the master branch is key (since this issue only hit sqlboiler users on Go 1.13).

What's confusing is that you'd expect lib/pq to pick up the decomposer interface implementation of the underlying decimal and use it correctly. I was hoping to see if you had any insights as to why this might not be working.

@ericlagergren
Copy link
Owner

So, this is what's up: golang/go#34315

The lib/pq package currently does not recognize the decomposer interface. I've opened an issue there: lib/pq#904

At first blush, I'm not sure whether this is a bug in lib/pq or the stdlib. If lib/pq adds support then the issue will be resolved, but the stdlib could also fix the issue by not having the conversion code cut out early if it sees a decimalDecomposer that's also a driver.Valuer.

I'm not sure there's much I could do on my end other than removing decimalDecomposer support, but it seems short sighted to do that because other packages like lib/pq are out of date. And I'm sure if I removed that interface I'd get a bug report asking me to add it back in. :)

Ideas?

@ericlagergren
Copy link
Owner

I'm gonna try opening a PR in lib/pq later this week. ericlagergren/pq@92294e9

@aarondl
Copy link
Author

aarondl commented Oct 10, 2019

Sounds good. As for now I've informed sqlboiler users to stick to tagged versions, which they should have been doing anyway. But go get makes it too convenient to not do that and many people are still not using go modules (for very good reasons mind you).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants