Skip to content

SELECT FLOAT to float64 #308

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

Closed
wants to merge 9 commits into from
Closed

SELECT FLOAT to float64 #308

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
064acea
Test case for wrong float64 value after selecting it from FLOAT table…
arvenil Feb 1, 2015
fc1c08e
Bugfix for selecting FLOAT to float64
arvenil Feb 1, 2015
38e6bd4
Another failing case is for non-prepared queries with parseTime=true
arvenil Feb 1, 2015
af12796
And fix for it
arvenil Feb 1, 2015
855d2cf
Well without &parseTime=true it doesn't work too
arvenil Feb 1, 2015
0a6d62a
And hacky fix for it
arvenil Feb 1, 2015
ab09bce
Funny thing, that not prepared statement without IFNULL workerd witho…
arvenil Feb 1, 2015
89b0d92
Let's be sure prepared statements also work with parseTime=true
arvenil Feb 1, 2015
6b012d3
Don't convert float32 to float32
arvenil Feb 9, 2015
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
41 changes: 41 additions & 0 deletions driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,47 @@ func TestTimezoneConversion(t *testing.T) {
}
}

func TestSelectFloatToFloat64(t *testing.T) {
createTest := func(query string, args ...interface{}) func(dbt *DBTest) {
return func(dbt *DBTest) {
v := float64(0.050551)

// Create table
dbt.mustExec("CREATE TABLE test (f FLOAT)")
dbt.mustExec("INSERT INTO test VALUE (?)", v)

// Retrieve
rows := dbt.mustQuery(query, args...)
if !rows.Next() {
dbt.Fatal("Didn't get any rows out")
}

var f float64
err := rows.Scan(&f)
if err != nil {
dbt.Fatal("Err", err)
}

// Check that dates match
if f != v {
dbt.Errorf("Float values don't match.\n")
dbt.Errorf(" Inserted: %v\n", v)
dbt.Errorf(" Selected: %v\n", f)
}
}
}

dsns := []string{
Copy link
Member

Choose a reason for hiding this comment

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

parseTime is irrelevant here I guess. You don't need to test both versions of the DSN. Just leave out parseTime altogether.

dsn + "&parseTime=true",
dsn + "&parseTime=false",
}
for _, testdsn := range dsns {
runTests(t, testdsn, createTest("SELECT f FROM test")) // not prepared statement
runTests(t, testdsn, createTest("SELECT f FROM test WHERE 1=?", 1)) // prepared statement
runTests(t, testdsn, createTest("SELECT IFNULL(f, 0) f FROM test")) // not prepared statement with IFNULL
}
}

// Special cases

func TestRowsClose(t *testing.T) {
Expand Down
38 changes: 24 additions & 14 deletions packets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"fmt"
"io"
"math"
"strconv"
"time"
)

Expand Down Expand Up @@ -616,22 +617,31 @@ func (rows *textRows) readRow(dest []driver.Value) error {
pos += n
if err == nil {
if !isNull {
Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be any edits necessary in readRow at all.
MySQL has basically two protocols: A binary protocol (where the bug seems to appear) and a text protocol. readRow is for the text protocol. Here the MySQL server just sends everything (hopefully) correctly formated as a string.

We just check for mc.parseTime here because with the DSN param parseTime=true the users asks us to return time values as time.Time, not as a string. Since the database/sql package can't parse it, we have to help here. Also kind of a dirty workaround.

Please try if it works properly if you don't change here anything at all. According to your bug description it should work.

if !mc.parseTime {
// @todo hacky fix, please make it better
if i > len(rows.columns)-1 {
continue
} else {
switch rows.columns[i].fieldType {
case fieldTypeTimestamp, fieldTypeDateTime,
fieldTypeDate, fieldTypeNewDate:
dest[i], err = parseDateTime(
string(dest[i].([]byte)),
mc.cfg.loc,
)
if err == nil {
continue
}
default:
}
switch rows.columns[i].fieldType {
case fieldTypeTimestamp, fieldTypeDateTime,
fieldTypeDate, fieldTypeNewDate:
if !mc.parseTime {
continue
}
dest[i], err = parseDateTime(
string(dest[i].([]byte)),
mc.cfg.loc,
)
if err == nil {
continue
}
case fieldTypeFloat:
val, err := strconv.ParseFloat(string(dest[i].([]byte)), 32)
dest[i] = float32(val)
if err == nil {
continue
}
default:
continue
}

} else {
Expand Down Expand Up @@ -1037,7 +1047,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.

math.Float32frombits already returns float32

pos += 4
continue

Expand Down