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: unsupported Scan when storing driver.Value type time.Time into type *sql.RawBytes #20746

Closed
aseemk opened this issue Jun 21, 2017 · 4 comments

Comments

@aseemk
Copy link

@aseemk aseemk commented Jun 21, 2017

Please answer these questions before submitting your issue. Thanks!

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

1.8.1

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

linux/amd64

What did you do?

Tried to Scan a row containing a timestamp column (from Postgres) into a *sql.RawBytes type for that column.

(Sorry, sharing a Play link isn't possible due to DB setup, and I also can't access the sql package-internal convertAssign method directly to illustrate. But if this isn't obvious, happy to try to provide a more formal repro elsewhere.)

What did you expect to see?

For it to work, i.e. no error. Scanning into []byte works, and my understanding of sql.RawBytes from #2698 is it's the same, just avoids a copy.

What did you see instead?

sql: Scan error on column index 3: unsupported Scan, storing driver.Value type time.Time into type *sql.RawBytes (the column index is of course arbitrary and specific to my usage)

Indeed, no handling of *sql.RawBytes, even though []byte is supported:
https://github.com/golang/go/blob/go1.8.3/src/database/sql/convert.go#L170-L181

I found one workaround: define a dummy Scanner type. E.g. go-gorp/gorp#277

So I can do that for now, but wanted to file this after spending some time debugging. Thanks!

@tejasmanohar
Copy link

@tejasmanohar tejasmanohar commented Jun 24, 2017

Would it make sense to support converting time.Time into sql.RawBytes? RawBytes is intended for scenarios where you want to original memory location of the byte array, but time.Time#Format is going to inevitably make a copy. I think this is the same reason you can't convert string into RawBytes.

@odeke-em odeke-em changed the title sql: unsupported Scan, storing driver.Value type time.Time into type *sql.RawBytes database/sql: unsupported Scan when storing driver.Value type time.Time into type *sql.RawBytes Jul 21, 2017
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Jul 21, 2017

I'll also tag the database/sql folks @kardianos @bradfitz

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 28, 2017

Change https://golang.org/cl/66830 mentions this issue: database/sql: convertAssign string and time.Time into RawBytes

@SchumacherFM
Copy link
Contributor

@SchumacherFM SchumacherFM commented Sep 28, 2017

I ran into the same problem. I have a package which scans all different types of columns into a resuable slice of []*sql.RawBytes. This works very well and avoids lots of allocs. But once a SQL column has the time.Time type, the scan into RawBytes fails of course. A real deal breaker.

I've implemented the fixes in convertAssign for scanning time.Time into RawBytes and also another switch case for string to RawBytes.

The conversion of time.Time into RawBytes uses no allocation. This gets checked in the test TestRawBytesAllocs.

The string->Rawbytes got a nice speed up due to the avoidance of reflection. Below is the benchmark.

Current implementation via reflection:

$ go test -v -run=🤐 -bench=Conve -count=2 -benchmem -timeout=17m
goos: darwin
goarch: amd64
pkg: database/sql
BenchmarkConvertAssign/stringRawBytes-4         	20000000	        61.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkConvertAssign/stringRawBytes-4         	20000000	        61.9 ns/op	      16 B/op	       1 allocs/op
PASS
ok  	database/sql	2.618s

New implementation via the additional switch case:

$ go test -v -run=🤐 -bench=Conve -count=2 -benchmem -timeout=17m
goos: darwin
goarch: amd64
pkg: database/sql
BenchmarkConvertAssign/stringRawBytes-4         	30000000	        49.2 ns/op	      16 B/op	       1 allocs/op
BenchmarkConvertAssign/stringRawBytes-4         	30000000	        48.3 ns/op	      16 B/op	       1 allocs/op
PASS
ok  	database/sql	3.447s

This is the Benchmark function (not included in the CL):

func BenchmarkConvertAssign(b *testing.B) {
	b.Run("stringRawBytes", func(b *testing.B) {
		src := "plain-str"
		dest := make(RawBytes, 0, len(src))
		for i := 0; i < b.N; i++ {
			if err := convertAssign(&dest, src); err != nil {
				b.Fatal(err)
			}
			if src != string(dest) {
				b.Fatalf("Want %q Have %q", src, dest)
			}
			dest = dest[:0]
		}
	})
}

The CL is https://go-review.googlesource.com/c/go/+/66830

cc @kardianos

@gopherbot gopherbot closed this in 3487c4e Oct 1, 2017
@golang golang locked and limited conversation to collaborators Oct 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.