[go] database/sql: add int64, float64, and bool fast paths to convertAssignRows
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded the change for review![]( "Open in Gerrit")
Commit message
database/sql: add int64, float64, and bool fast paths to convertAssignRows
convertAssignRows has fast paths for string, []byte, and time.Time
sources, but not for int64, float64, and bool. Scanning an int64 into
*int64 went through reflection, and into *int through a decimal
string round-trip (strconv.FormatInt then strconv.ParseInt),
allocating for any value >= 100.
Add fast paths for these three types into their exact-type
destinations (*int64, *int, *float64, *bool). The *int arm falls
back to the existing conversion on 32-bit overflow, so error text
is unchanged. Named types and Scanner destinations are unaffected
since type switches match exact types only.
One behavior change: scanning a bool into a typed-nil *bool now
returns errNilPtr instead of panicking, consistent with the other arms.
Add conversion tests for the int64 arms, which had no direct
coverage.
Expands on CL 622598 by Charlie Vieth, additionally handling int64
to *int and adding the benchmarks requested in review.
goos: darwin
goarch: arm64
pkg: database/sql
cpu: Apple M2 Pro
│ old │ new │
│ sec/op │ sec/op vs base │
ConvertAssignRows/Int64ToInt64-10 16.870n ± 1% 2.415n ± 1% -85.68% (p=0.000 n=25)
ConvertAssignRows/Int64ToInt-10 52.990n ± 1% 2.268n ± 1% -95.72% (p=0.000 n=25)
ConvertAssignRows/Int64ToNullInt64-10 24.150n ± 1% 7.815n ± 1% -67.64% (p=0.000 n=25)
ConvertAssignRows/Float64ToFloat64-10 16.900n ± 1% 2.256n ± 1% -86.65% (p=0.000 n=25)
ConvertAssignRows/BoolToBool-10 4.823n ± 0% 2.260n ± 1% -53.14% (p=0.000 n=25)
ConvertAssignRows/Int64ToInt32-10 45.93n ± 0% 44.04n ± 0% -4.11% (p=0.000 n=25)
ConvertAssignRows/Int64ToAny-10 4.200n ± 1% 3.009n ± 0% -28.36% (p=0.000 n=25)
ConvertAssignRows/StringToInt64-10 37.54n ± 1% 36.88n ± 1% -1.76% (p=0.000 n=25)
ConvertAssignRows/BytesToBytes-10 16.01n ± 2% 15.68n ± 1% -2.06% (p=0.000 n=25)
ConvertAssignRows/BytesToRawBytes-10 2.678n ± 1% 2.564n ± 0% -4.26% (p=0.000 n=25)
ConvertAssignRows/BytesToInt64-10 46.95n ± 0% 46.51n ± 1% -0.94% (p=0.011 n=25)
ConvertAssignRows/NilToAny-10 1.970n ± 1% 1.976n ± 1% ~ (p=0.378 n=25)
ConvertAssignRows/BytesToString-10 11.35n ± 1% 11.21n ± 1% -1.23% (p=0.036 n=25)
ConvertAssignRows/StringToString-10 2.931n ± 0% 2.674n ± 0% -8.77% (p=0.000 n=25)
ConvertAssignRows/TimeToTime-10 2.599n ± 1% 2.635n ± 1% +1.39% (p=0.000 n=25)
geomean 11.03n 5.837n -47.09%
B/op and allocs/op are unchanged except Int64ToInt, which drops from
16 B, 1 alloc to 0.
Fixes #80671
Co-authored-by: Charlie Vieth <charli...@gmail.com>
Change diff
diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index e965d09..2c91495 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -247,6 +247,42 @@
func convertAssignRows(dest, src any, rows *Rows) error {
// Common cases, without reflect.
switch s := src.(type) {
+ case int64:
+ switch d := dest.(type) {
+ case *int64:
+ if d == nil {
+ return errNilPtr
+ }
+ *d = s
+ return nil
+ case *int:
+ if d == nil {
+ return errNilPtr
+ }
+ if int64(int(s)) == s {
+ *d = int(s)
+ return nil
+ }
+ // Out of range for int, let the generic path produce the error.
+ }
+ case float64:
+ switch d := dest.(type) {
+ case *float64:
+ if d == nil {
+ return errNilPtr
+ }
+ *d = s
+ return nil
+ }
+ case bool:
+ switch d := dest.(type) {
+ case *bool:
+ if d == nil {
+ return errNilPtr
+ }
+ *d = s
+ return nil
+ }
case string:
switch d := dest.(type) {
case *string:
diff --git a/src/database/sql/convert_test.go b/src/database/sql/convert_test.go
index 10ae9ac..6a0be53 100644
--- a/src/database/sql/convert_test.go
+++ b/src/database/sql/convert_test.go
@@ -54,6 +54,7 @@
scanbytes []byte
scanraw RawBytes
scanint int
+ scanint64 int64
scanuint8 uint8
scanuint16 uint16
scanbool bool
@@ -71,6 +72,9 @@
// Exact conversions (destination pointer type matches source type)
{s: "foo", d: &scanstr, wantstr: "foo"},
{s: 123, d: &scanint, wantint: 123},
+ {s: int64(123), d: &scanint64, wantint: 123},
+ {s: int64(123), d: &scanint, wantint: 123},
+ {s: int64(-123), d: &scanint, wantint: -123},
{s: someTime, d: &scantime, wanttime: someTime},
// To strings
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 1763b3b..e785961 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -5774,3 +5774,147 @@
}
return ConvertAssign(ctx, dest, c.row[index])
}
+
+func BenchmarkConvertAssignRows(b *testing.B) {
+ now := time.Now()
+ b.Run("Int64ToInt64", func(b *testing.B) {
+ var d int64
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, int64(42), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("Int64ToInt", func(b *testing.B) {
+ var d int
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, int64(1234567890123), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("Int64ToNullInt64", func(b *testing.B) {
+ var d NullInt64
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, int64(42), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("Float64ToFloat64", func(b *testing.B) {
+ var d float64
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, float64(3.14159), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("BoolToBool", func(b *testing.B) {
+ var d bool
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, true, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("Int64ToInt32", func(b *testing.B) {
+ var d int32
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, int64(123456), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("Int64ToAny", func(b *testing.B) {
+ var d any
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, int64(42), nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("StringToInt64", func(b *testing.B) {
+ var d int64
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, "1234567890123", nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("BytesToBytes", func(b *testing.B) {
+ var d []byte
+ var src any = []byte("alice-example-name") // box once: per-op boxing would add a phantom alloc
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, src, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("BytesToRawBytes", func(b *testing.B) {
+ var d RawBytes
+ var src any = []byte("alice-example-name")
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, src, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("BytesToInt64", func(b *testing.B) {
+ var d int64
+ var src any = []byte("1234567890123")
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, src, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("NilToAny", func(b *testing.B) {
+ var d any
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, nil, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("BytesToString", func(b *testing.B) {
+ var d string
+ var src any = []byte("alice-example-name")
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, src, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("StringToString", func(b *testing.B) {
+ var d string
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, "alice-example-name", nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+ b.Run("TimeToTime", func(b *testing.B) {
+ var d time.Time
+ var src any = now
+ b.ReportAllocs()
+ for range b.N {
+ if err := convertAssignRows(&d, src, nil); err != nil {
+ b.Fatal(err)
+ }
+ }
+ })
+}
Change information
- M src/database/sql/convert.go
- M src/database/sql/convert_test.go
- M src/database/sql/sql_test.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Gopher Robot added 1 comment![]( "Open in Gerrit")
I spotted some possible problems with your PR:
1. It looks like you have a properly formated bug reference, but the convention is to put bug references at the bottom of the commit message, even if a bug is also mentioned in the body of the message.
Please address any problems by updating the GitHub PR.
When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above. These findings are based on heuristics; if a finding does not apply, briefly reply here saying so.
To update the commit title or commit message body shown here in Gerrit, you must edit the GitHub PR title and PR description (the first comment) in the GitHub web interface using the 'Edit' button or 'Edit' menu entry there. Note: pushing a new commit to the PR will not automatically update the commit message used by Gerrit.
For more details, see:
- [how to update commit messages](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message) for PRs imported into Gerrit.
- the Go project's [conventions for commit messages](https://go.dev/doc/contribute#commit_messages) that you should follow.
(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Teather (Gerrit)
David Teather added 1 comment![]( "Open in Gerrit")
I spotted some possible problems with your PR:
1. It looks like you have a properly formated bug reference, but the convention is to put bug references at the bottom of the commit message, even if a bug is also mentioned in the body of the message.Please address any problems by updating the GitHub PR.
When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above. These findings are based on heuristics; if a finding does not apply, briefly reply here saying so.
To update the commit title or commit message body shown here in Gerrit, you must edit the GitHub PR title and PR description (the first comment) in the GitHub web interface using the 'Edit' button or 'Edit' menu entry there. Note: pushing a new commit to the PR will not automatically update the commit message used by Gerrit.
For more details, see:
- [how to update commit messages](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message) for PRs imported into Gerrit.
- the Go project's [conventions for commit messages](https://go.dev/doc/contribute#commit_messages) that you should follow.
(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)
The Fixes line is at the bottom of the body, only Co-authored-by trailer follows it.
Modeled it on https://github.com/golang/go/commit/04dc12c1a1 so I don't think any change is needed. Let me know if I'm mistaken
Related details
- Brad Fitzpatrick
- Daniel Theophanes
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emmanuel Odeke (Gerrit)
Emmanuel Odeke voted and added 3 comments![]( "Open in Gerrit")
Votes added by Emmanuel Odeke
| Commit-Queue | +1 |
3 comments
Thank you for this change, David! Much appreciated, and I've added a suggestion to prevent this change from breaking code out there.
One behavior change: scanning a bool into a typed-nil *bool now
returns errNilPtr instead of panicking, consistent with the other arms.Kindly please revert this behavioral change, because it is sudden and needs much more discussion to see whose code breaks and it is totally unrelated to this change.
case bool:
switch d := dest.(type) {
case *bool:
if d == nil {
return errNilPtr
}
*d = s
return nil
}Kindly please keep this CL focused on int664, float64 otherwise this change for bool to return an error instead of panicking perhaps needs a proposal change because it would be unwieldly breaking for users out there. Thank you.
Related details
- Brad Fitzpatrick
- Damien Neil
- Daniel Theophanes
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |