-
-
Notifications
You must be signed in to change notification settings - Fork 233
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
fix: support scan float32 to float32 #1088
base: master
Are you sure you want to change the base?
Conversation
Do we have any specific reason not to do this? I suspect this might be a bug. |
@j2gg0s case float32:
dest.SetFloat(float64(src))
return nil
|
@Re3os To me, there is no difference in merit between adding an if condition and duplicating code. |
But why? You allow to scan float64 into float32 which results in precision loss, but want to forbid scanning float32 to float64 which is fine. I agree with @Re3os that this could be refactored into a more general function:
AFAIR no I think I assumed that database/sql never returns float32. |
281274e
to
a52e733
Compare
func scanFloat(dest reflect.Value, src interface{}, bits int) error {
var parsedFloat float64
switch src := src.(type) {
case nil:
parsedFloat = 0
case float32:
parsedFloat = float64(src)
case float64:
parsedFloat = src
case []byte:
f, err := strconv.ParseFloat(string(src), bits)
if err != nil {
return err
}
parsedFloat = f
case string:
f, err := strconv.ParseFloat(src, bits)
if err != nil {
return err
}
parsedFloat = f
default:
return scanError(dest.Type(), src)
}
if bits == 32 && dest.Kind() == reflect.Float32 {
dest.SetFloat(float64(float32(parsedFloat)))
} else if bits == 64 && dest.Kind() == reflect.Float64 {
dest.SetFloat(parsedFloat)
} else {
return scanError(dest.Type(), src)
}
return nil
} float32 = 32bits Didn't test it, no time func scanFloat32(dest reflect.Value, src interface{}) error {
return scanFloat(dest, src, 32)
}
func scanFloat64(dest reflect.Value, src interface{}) error {
return scanFloat(dest, src, 64)
} |
Current logic in master:
I think: Due to personal habits, I am unwilling to modify any existing logic unless I have confirmed how everyone uses it, or it is obviously incorrect. However, I think it is also acceptable to directly support 3 & 4 now. |
Close #1087