-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: provide a way for a driver to get its own driver.Rows struct from sql.Row and sql.Rows #5606
Comments
Labels changed: added priority-later, removed priority-triage. Owner changed to @bradfitz. Status changed to Accepted. |
@arnehormann My problem is with the proposed Field interface is that it is MySQL-specific. Although that MysqlType, MysqlDeclaration could be renamed to SqlType, SqlDeclaration for a little bit more agnostic. Just as IsBinary, IsPrimaryKey, IsUniqueKey, IsMultipleKey, IsAutoinrement has no meaning in Oracle. I'd like to make this as future-proof or at least extensible as possible - for example, Oracle can return cursor in a resultset, which results in "false" in every "IsXxx" method in this interface. That's why I think a simple string can provide the needed info: SqlType return "VARCHAR2", SqlDeclaration return "VARCHAR2(32) NOT NULL" - as the database provides it. Maybe not important, but I think only database.sql.Rows should be inspectable: that's a bad practice to inspect every Row, and unneeded, too. |
@ tgulacsi78 it's not MySQL specific at all. I used MySQL as an example and the Field interface would live in the mysql driver (or the pg driver, or the odbc driver, or ...) and expose the information available to that specific driver. That part of my proposal was nothing but a usage example, the changes I want to have in go are described at the top of my post and in this issue. That's something I even actively tried to avoid when designing this, I want to have next to all functionality in the driver and only change database/sql so the driver can access it. You are the third one to make that remark, I must have been unclear in my proposal. Any suggestions how I can correct it? |
My problem is ORM scaffolding. I want to generate code with structs and support functions to call Scan on them for a MySQL database. Getting information by calling SHOW COLUMNS or SHOW CREATE TABLE for existing tables is possible, but I want to also support user provided queries and structs containing data from SHOW queries. With a lot of boilerplate code, I could add some of this myself. Still, I have no idea how I can get column type information with the current database/sql in all cases I listed. For SHOW queries, adding them by hand by reading the manual will require a lot of code and quite some time in the depth of the MySQL documentation (including which SHOW queries to leave out because they were added in Version X or added two new columns in version Y). But knowing the types used in user specified queries is still harder. For JOINs, it can probably be done by parsing the SQL and inspecting the table structure with SHOW queries. Concerning aggregation functions, I have no idea what approach I should take. |
I should probably add that I am more than willing to prepare a CL myself, I only want to know if this is eligible for inclusion first. I'm not saying it is impossible to get what I want, I can get there with unsafe. I'd just prefer to have a way without unsafe that is fit for inclusion in the drivers and eases the maintenance burden. I built such a workaround with support code usable by other drivers in a separate packages for github.com/go-sql-driver/mysql: https://github.com/arnehormann/sqlinternals/tree/master/mysqlinternals I didn't evaluate it yet, but I think this can help users cut down the amount of conversions and probably also allocations. I also see potential for marshalling (JSON could store fields as numbers instead of strings without a check). Thinking about it, I also see another, more direct way to get column information: Change the return value of Columns() from []string to []Stringer and pass values provided by the driver. The driver can provide an interface tailored to its database the Stringers can be cast to. That would not cause an api addition but a go fix repairable change, though all drivers would have to change as well. I don't think it's possible now under the Go1 guarantee as it's not really a bug but more a shortcoming IMO, but maybe for Go2... |
Comment 10 by maciek@heroku.com: For what it's worth, this could also help create a more idiomatic LISTEN/NOTIFY [1] interface, and allow a COPY [2] interface, for pq. See the discussion in this pq pull request [3] and the note in the golang-nuts thread [4]. [1]: http://www.postgresql.org/docs/current/static/sql-listen.html and http://www.postgresql.org/docs/current/static/sql-notify.html [2]: http://www.postgresql.org/docs/current/static/sql-copy.html [3]: lib/pq#106 [4]: https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/eZmWzURD-Uo |
Issue #6345 has been merged into this issue. |
I'd like for this to be added in Go 1.3. It provides missing reflection capabilities to the driver, gives users knowledge about what column types to expect before Scan is called the first time and enables usage of the driver in unknown contexts not coupled to a known database - e.g. for generic tools. As I wrote before in #9, I'm willing to implement it myself and post a CL if the design I outlined is acceptable. The design is described in the first post in https://groups.google.com/forum/#!topic/golang-nuts/2aLctcVyp6Q, |
LGTM, but thats only 2c. Although a simple string repredentation of the column typed would also be enough, and tjat way we don't need type assertions, just text parsing. And the driver would know how to interpret those strings. Anxway, anything is better than this lack of information. 2013.12.09. 23:09 ezt írta ( <go@googlecode.com>): |
I'll take another shot at this - seems not to be my strong suit but I'm very willing to learn... Currently, database/sql provides access to column names with Columns. The drivers may have access to additional information, e.g. the column type, creation comment, character set and collation, precision for decimal types, ... After the driver's native row iterator (driver.Rows) is stored in sql.Rows or sql.Row, the available information except for the names cannot be accessed any longer, but it is useful for applications: * deriving the SQL needed for ALTER TABLE statements when a column should be changed * using the closest matching type when data is serialized (e.g. numbers in JSON) * formatting table columns for an html export by column type * generating code to scan into structs from any query * exploring unknown databases in tools not coded for a specific database With the changes proposed in the issue, drivers can provide functions like func ColumnTypes(rows *sql.Rows) []reflect.Type func ColumnSql(rows *sql.Rows) []string The proposed change intends to enable drivers to add such functions to a driver but to hide the native driver implementation from everything but the driver and to keep the api of database/sql as is. The only way to do this right now uses unsafe to access the rowsi field in sql.Rows. Adding all the functions to database/sql is the wrong approach because the available informations varies by used DBMS. |
So you're proposing: package sql func (rs *Rows) DriverRows() driver.Rows { return rs.rowsi } ? If this will be driver-specific anyway, that isn't very motivating. I would rather add something generic like: func (rs *Rows) Columns() ([]Column, error) And have drivers decide whether they implement it and which fields of Column they populate. Then it works for all drivers in the same way at least, even if with different levels of support. |
No, I'm not. The Column one gets close, but I think that's too limiting, too. What I want is to move the whole column metadata access to the drivers. Then, anyone can import a driver for the specific database he/she uses (no longer as _ but with a named import). To support this, the driver could provide a DriverColumn type with access to all available column information and a function Columns(*sql.Rows) ([]DriverColumn, error) In that function, it must get its own database/sql/driver.Rows implementation back to access the available data as it cannot access rowsi. That's my actual problem, this is impossible as is without unsafe. But it would be easy in database/sql - if there were a function to extract (unwrap) the rowsi field from *Rows or *Row. To avoid a maintenance hassle, I don't want this function (henceforth "unwrapper") or access to rowsi to be exported, I want to restrict it to the driver. The best way to do so is to return it on Registration; then, it's available on the first query. The Go compatibility promise forbids making it a return value of Register. So we introduce a new interface in driver with a SetUnwrapper(unwrapper) method and call that on all drivers implementing it. In that function, the driver stores the unwrapper in a variable and can use it in the Columns() function I described above. The interactions / responsibilities in my proposal: - Roles - sql: database/sql driver: database/sql/driver dbx: implementation of database/sql/driver (for me, github.com/go-sql-driver/mysql) driver.CanUnwrap: an interface with SetUnwrapper(unwrapper) - The registration process - <dbx> calls sql.Register in its init() function <sql> checks if <dbx> implements driver.CanUnwrap and calls SetUnwrapper(unwrapper), where unwrapper is an unexported function in <sql> that returns rowsi <dbx>.SetUnwrapper receives the unwrapper and stores it in a var - Query - db, _ := sql.Open("dbx", "user@tcp(localhost:3306)/") rows, _ := sql.Query("SELECT col1, col2 FROM whatever") cols, _ := dbx.Columns(rows) // <- everything in the driver - as long as it can get its native data structures back! // cols is []dbx.Column - and provides access to all available data for dbx |
I think the benefits of this approach very much outweight the cost of understanding the (rather strange) callback approach neccesitated by the compatibility guarantee and keeping access restrictions to the native types. And... this only concerns driver implementers and I don't think any of them are hard to reach or unwilling to discuss this stuff. So it's not that much of a problem. |
For reference (and reviews), I mailed my proposal as https://golang.org/cl/43510044/ it's only different in that it uses "Inspect" instead of "Unwrap". The top part of fakedb_test.go shows the change drivers have to make to use it. The changes in sql_test.go illustrate how it could or should be used. It's just the missing enabler, what drivers _can_ do with it is not part of the CL. http://godoc.org/github.com/arnehormann/sqlinternals/mysqlinternals is an example of what could be gained for MySQL, it currently depends on unsafe. This could also still be improved upon, currently the mysql driver only stores part of the useable information as it cannot be used and we won't change that until it becomes available without unsafe. |
CL https://golang.org/cl/43510044 references this issue. |
Any updates? |
I am also interested in this. Just found this thread after trying to get access to the native row implementation. It seems to me that the hack to https://github.com/arnehormann/sqlinternals/blob/e3f9da33ba6e09e023c4b1d32ea73b39d3132d59/unsafe.go#L102 inspect the value is a bad solution. I would rather fork the database/sql and add in more interfaces. |
I find the interface a bit weird. My first instinct would have been for the drivers to implement a new method in the database/sql/driver.Rows object they're returning. This method would return an interface{} which the user of database/sql would then assert to structs or interfaces defined by the driver. This could be nicer for backwards compatibility as well, since you could check whether the returned object implements a certain interface added later in the driver's life cycle. |
Well for my purposes, I found github.com/kdar/dbtogo to generate the go schema for my tables. With that schema generated I know the types. Then I am using upperdb https://upper.io/db as a replacement for the standard go driver. It all works out OK. |
This looks duplicate of #16652 . Recommend closing this issue. |
by arnehormann:
The text was updated successfully, but these errors were encountered: