ColumnTypes() improvement. #22
Conversation
it was impossible to extract column types with Rows.ColumnTypes() function.
but Crate's JSON response already contains column type information(col_types).
this commit contains implement `col_types`.
Look for following example codes.
Example Code:
package main
import (
"fmt"
"database/sql"
_ "go-crate"
)
func main() {
db, _ := sql.Open("crate", "http://127.0.0.1:4200")
rows, _ := db.Query("select col1, col2 from tbl")
types, _ := rows.ColumnTypes()
for i, l := 0, len(types); i < l; i++ {
t := types[i]
fmt.Println(t.Name(), ":", t.DatabaseTypeName())
}
}
|
Hello, @nicetip! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
| 14: "geo_shape", | ||
| } | ||
|
|
||
| func (r *Rows) ColumnTypeDatabaseTypeName(index int) string { |
There was a problem hiding this comment.
exported method Rows.ColumnTypeDatabaseTypeName should have comment or be unexported
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/herenow/go-crate/pulls/22. |
| @@ -191,6 +193,34 @@ func (r *Rows) Close() error { | |||
| return nil | |||
| } | |||
|
|
|||
| var typeMap = map[int]string{ | |||
| 2: "byte", | |||
There was a problem hiding this comment.
Just one question, Go's documentation specifies that types should be returned uppercased, here:
https://golang.org/pkg/database/sql/driver/#RowsColumnTypeDatabaseTypeName
Also here is the discussion about this feature.
Crate documents all of their types lowercased, I see no reason for us to uppercase them.
Before merging this, I would like to know if you have any thoughts on this.
There was a problem hiding this comment.
hello? @herenow
I didn't know there were a spec for upper/lowercase.
I just used lowercase cause crate's document was written in lowercase and it's easy to type.
I thought about this for a while and looked for other golang drivers implementations.
Other drivers also already knows about this and implemented in uppercase, even commented it should be uppercase.
So I think uppercase would be better, cause somebody who knows about this spec, may expect that function will return uppercased.
And others may not cares about it. : )
Thanks you for asking my opinion.
it was impossible to extract column types with Rows.ColumnTypes() function.
but Crate's JSON response already contains column type information(col_types).
this commit contains implement
col_types.Look for following example codes.
Example Code:
package main
import (
"fmt"
"database/sql"
_ "go-crate"
)
func main() {
db, _ := sql.Open("crate", "http://127.0.0.1:4200")
rows, _ := db.Query("select col1, col2 from tbl")
types, _ := rows.ColumnTypes()
for i, l := 0, len(types); i < l; i++ {
t := types[i]
fmt.Println(t.Name(), ":", t.DatabaseTypeName())
}
}