Skip to content

test and fix for MysSQL float parsing into float64 when placeholders are used #434

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 11, 2016
Merged

test and fix for MysSQL float parsing into float64 when placeholders are used #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e81889
test and fix for MysSQL float parsing into float64 when placeholders …
pib Mar 3, 2016
f47acb0
Add myself to AUTHORS
pib Mar 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Lucas Liu <extrafliu at gmail.com>
Luke Scott <luke at webconnex.com>
Michael Woolnough <michael.woolnough at gmail.com>
Nicola Peduzzi <thenikso at gmail.com>
Paul Bonser <misterpib at gmail.com>
Runrioter Wung <runrioter at gmail.com>
Soroush Pour <me at soroushjp.com>
Stan Putrya <root.vagner at gmail.com>
Expand Down
48 changes: 47 additions & 1 deletion driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func TestInt(t *testing.T) {
})
}

func TestFloat(t *testing.T) {
func TestFloat32(t *testing.T) {
runTests(t, dsn, func(dbt *DBTest) {
types := [2]string{"FLOAT", "DOUBLE"}
in := float32(42.23)
Expand All @@ -388,6 +388,52 @@ func TestFloat(t *testing.T) {
})
}

func TestFloat64(t *testing.T) {
runTests(t, dsn, func(dbt *DBTest) {
types := [2]string{"FLOAT", "DOUBLE"}
var expected float64 = 42.23
var out float64
var rows *sql.Rows
for _, v := range types {
dbt.mustExec("CREATE TABLE test (value " + v + ")")
dbt.mustExec("INSERT INTO test VALUES (42.23)")
rows = dbt.mustQuery("SELECT value FROM test")
if rows.Next() {
rows.Scan(&out)
if expected != out {
dbt.Errorf("%s: %g != %g", v, expected, out)
}
} else {
dbt.Errorf("%s: no data", v)
}
dbt.mustExec("DROP TABLE IF EXISTS test")
}
})
}

func TestFloat64Placeholder(t *testing.T) {
runTests(t, dsn, func(dbt *DBTest) {
types := [2]string{"FLOAT", "DOUBLE"}
var expected float64 = 42.23
var out float64
var rows *sql.Rows
for _, v := range types {
dbt.mustExec("CREATE TABLE test (id int, value " + v + ")")
dbt.mustExec("INSERT INTO test VALUES (1, 42.23)")
rows = dbt.mustQuery("SELECT value FROM test WHERE id = ?", 1)
if rows.Next() {
rows.Scan(&out)
if expected != out {
dbt.Errorf("%s: %g != %g", v, expected, out)
}
} else {
dbt.Errorf("%s: no data", v)
}
dbt.mustExec("DROP TABLE IF EXISTS test")
}
})
}

func TestString(t *testing.T) {
runTests(t, dsn, func(dbt *DBTest) {
types := [6]string{"CHAR(255)", "VARCHAR(255)", "TINYTEXT", "TEXT", "MEDIUMTEXT", "LONGTEXT"}
Expand Down
2 changes: 1 addition & 1 deletion packets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,7 @@ func (rows *binaryRows) readRow(dest []driver.Value) error {
continue

case fieldTypeFloat:
dest[i] = float64(math.Float32frombits(binary.LittleEndian.Uint32(data[pos : pos+4])))
dest[i] = float32(math.Float32frombits(binary.LittleEndian.Uint32(data[pos : pos+4])))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the driver spec does not allow to return a float32 as a driver.Value: https://golang.org/pkg/database/sql/driver/#Value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, it's strange that it works anyway. I suppose what the spec says and the details of the implementation vary a bit.

Looking through some of the standard library sql code, I see that they seem to work around this issue by converting floats to strings and back using strconv.ParseFloat with the bit size of the field it is going into: https://golang.org/src/database/sql/convert.go#L256

Actually, that function is called by sql.Rows.Scan, which means that their spec for Value is actually inaccurate: https://golang.org/src/database/sql/sql.go#L1849

It seems that the doc for driver.Rows https://golang.org/pkg/database/sql/driver/#Rows should be updated to specify that they do handle more types, including float32.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I will submit a ticket to the golang project asking for clarification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works and we have proper tests for it, I would be fine with ignoring the specification (which is rather flawed anyway). We're testing against all Go versions and tip, so we would notice if it breaks in a future version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this change fixes the issue I was running into and the tests I added cover both cases and passed on Travis.

I added an issue to the Go repo so they will hopefully fix the documentation to include the full list of types that are actually allowed in returns from drivers.

pos += 4
continue

Expand Down