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/driver: Scan.Next documentation incorrect #6497
Labels
Milestone
Comments
I don't see the problem. Just because something happens to work, that doesn't mean it's guaranteed. I believe the implementation of database/sql and database/sql/driver are consistent with each other. I don't know the mgodbc codebase and whether it's correct or not. Please explain again what is wrong? I will be gone for two weeks, though, so likely won't reply until then. Status changed to WaitingForReply. |
Value returned by driver gets converted to value returned to user in convertAssign. Isn't it? Looking at the code, if both are strings, then no conversion is done. So that explains why strings returned by driver reach their user. That is, I would say, quite common scenario. And, I agree with you, perhaps documentation shouldn't be as strict. Even if things work, we might end up with extra memory allocations that otherwise aren't needed. I don't see your point about Scanner.Scan. Can you explain more? Alex |
Comment 6 by cpfohl@valetude.com: Sorry for the long pause, work required a work around, and I didn't have time to get back here. I'll explain what I was trying to do, perhaps I'm doing it wrong, thinking about it wrong, or something. Should have done that in the first place, programmers write the worst bug reports...I am a programmer. I want to take the results of a query and scan them into `map[string]interface{}`, where the key is the column name, and the value is the actual value at that location. This is handy for a variety of reasons, for me it's because I want to return the results of a query as JSON without an intermediate struct that will be 100% coupled to my query. Currently I'm doing this like so: http://play.golang.org/p/-vYciy-s9D It works great as long the source data is typed correctly, but if the source data comes from a driver which follows the 'convert to []byte' rule it fails, because deserializing []byte into interface{} results in the same thing when the original value was a varchar, blob, or fixed width char(n); heck, xml in sqlserver is a []byte. Does that make more sense? The example I passed along doesn't run because I'm not really sure how to fake a *sql.Rows...so if someone creates (or points me at instructions to create) the mockRows() func it'll better exemplify this for you. |
Comment 7 by Christopher.Pfohl: Sorry for the long pause, work required a work around, and I didn't have time to get back here. I'll explain what I was trying to do, perhaps I'm doing it wrong, thinking about it wrong, or something. Should have done that in the first place, programmers write the worst bug reports...I am a programmer. I want to take the results of a query and scan them into `map[string]interface{}`, where the key is the column name, and the value is the actual value at that location. This is handy for a variety of reasons, for me it's because I want to return the results of a query as JSON without an intermediate struct that will be 100% coupled to my query. Currently I'm doing this like so: http://play.golang.org/p/-vYciy-s9D It works great as long the source data is typed correctly, but if the source data comes from a driver which follows the 'convert to []byte' rule it fails, because deserializing []byte into interface{} results in the same thing when the original value was a varchar, blob, or fixed width char(n); heck, xml in sqlserver is a []byte. Does that make more sense? The example I passed along doesn't run because I'm not really sure how to fake a *sql.Rows...so if someone creates (or points me at instructions to create) the mockRows() func it'll better exemplify this for you. |
> ... but if the source data comes from a driver which follows the 'convert to []byte' rule it fails, because deserializing []byte into interface{} results in the same thing when the original value was a varchar, blob, or fixed width char(n); heck, xml in sqlserver is a []byte. Yes, you can change your (*Column).Scan to have: case []byte: c.Val = string(x) but this way you cannot distinguish between string columns and others (blobs, ...). > ... types that actually *are* strings actually wind up being []byte because the driver's obey the above documentation (see mgodbc for a case in point, line 916) I am not sure mgodbc is doing good here. string to []byte conversions and back are not free, you can look at generated asm (go build -ldflags "-a" ...) to see what happens under covers. Alex |
Comment 9 by Christopher.Pfohl: @Alex, if mgodbc is not doing good here, then it's doing bad because it's obeying the documentation (http://golang.org/pkg/database/sql/driver/#Rows) which says for type Rows, method Next: // Next is called to populate the next row of data into // the provided slice. The provided slice will be the same // size as the Columns() are wide. // // The dest slice may be populated only with // a driver Value type, but excluding string. // All string values must be converted to []byte. // // Next should return io.EOF when there are no more rows. Note the second to last line of text: "All string values must be converted to []byte" How is mgodbc doing bad? It's obeying exactly what the documentation says to do. |
> ... How is mgodbc doing bad? ... Most user programs will use "*string" in Scan to receive "string" database data. This is how this data gets around: 1) mgodbc receives that data as utf16 from ODBC; 2) mgodbc uses utf16ToString to convert it to "string"; 3) mgodbc converts it to "[]byte"; 4) database/sql converts that back to "string"; I think steps 3) and 4) are waste of computer resources. Alex |
> ... it doesn't do step 4, it leaves that to the user. ... Not the user, database/sql does that: https://code.google.com/p/go/source/browse/src/pkg/database/sql/convert.go#109 > ... mgodbc is doing *exactly* what the documentation says to do. Do we agree that the documentation is wrong? Yes, I don't understand many things in that doco. But I am not familiar with all design details. You have to convince Brad. Alex |
@Christopher, It seems that reading any "sql string" value into sql.RawByte or []byte fails, if database/sql/driver.Scan.Next returns string. I had to change my own driver to fix that. Would you like to review it: https://golang.org/cl/18270043/ ? Alex |
CL https://golang.org/cl/19439 mentions this issue. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by Christopher.Pfohl:
The text was updated successfully, but these errors were encountered: