Skip to content
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

Closed
gopherbot opened this issue May 31, 2013 · 27 comments
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented May 31, 2013

by arnehormann:

What is the expected output?

There should be a way to access the full information a driver has for result columns.
Workarounds are brittle and slow. They depend on the struct layout, require reflect or
unsafe and involve some duplication.

What do you see instead?

I can only get the names by calling Columns(). The rest of the result metadata can not
be retrieved.

Please provide any additional information below.

I posted a proposal on go-nuts:
https://groups.google.com/forum/#!topic/golang-nuts/2aLctcVyp6Q

In short, I propose to add a new interface to database/sql/driver that passes a callback
function to the driver.
The callback is returned after a type assertion in database/sql.Register.
It can be used to get the driver.Rows implementation given database/sql/Row[s].
The native Rows implementation by the driver is returned as interface{}.

Given this function, a driver can decide to expose more detailed metadata for the result
columns by implementing the interface and providing an exported function that retrieves
the metadata from *Row or *Rows.
@tgulacsi
Copy link
Contributor

@tgulacsi tgulacsi commented May 31, 2013

Comment 2:

Why not just add a way to let the driver return the type as strinf (just as the name) -
there are so many db variants...
@gopherbot
Copy link
Author

@gopherbot gopherbot commented May 31, 2013

Comment 3 by arnehormann:

The DB variants wouldn't be an issue for this proposal, as it's the drivers
responsibility to deal with the data - and there's only one DB the driver is written
for. Returning everything as a string would be slow and not very useful.
@robpike
Copy link
Contributor

@robpike robpike commented Jun 1, 2013

Comment 4:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@tgulacsi
Copy link
Contributor

@tgulacsi tgulacsi commented Jun 1, 2013

Comment 5:

@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.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jun 1, 2013

Comment 6 by arnehormann:

@ 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?
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 3, 2013

Comment 7:

Hi. I'm back from vacation and reading these threads & bugs now.
You proposed a solution, but what is the problem?
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jun 3, 2013

Comment 8 by arnehormann:

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.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jun 5, 2013

Comment 9 by arnehormann:

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...
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jun 6, 2013

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
@rsc
Copy link
Contributor

@rsc rsc commented Oct 18, 2013

Comment 11:

Issue #6345 has been merged into this issue.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 27, 2013

Comment 12:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 13:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 14:

Labels changed: added repo-main.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Dec 9, 2013

Comment 15 by arnehormann:

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,
@tgulacsi
Copy link
Contributor

@tgulacsi tgulacsi commented Dec 10, 2013

Comment 16:

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>):
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 14, 2014

Comment 17:

Can somebody summarize succinctly what the problem is here?

Owner changed to ---.

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jan 15, 2014

Comment 18 by arnehormann:

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.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 16, 2014

Comment 19:

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.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jan 16, 2014

Comment 20 by arnehormann:

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
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jan 16, 2014

Comment 21 by arnehormann:

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.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jan 22, 2014

Comment 22 by arnehormann:

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.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Apr 9, 2014

Comment 23:

CL https://golang.org/cl/43510044 references this issue.
@matrixik
Copy link

@matrixik matrixik commented Jan 22, 2015

Any updates?

@h4ck3rm1k3
Copy link

@h4ck3rm1k3 h4ck3rm1k3 commented Apr 17, 2015

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.
Am going to take a look at this module if it can be used.
https://github.com/flynn/go-sql

@johto
Copy link
Contributor

@johto johto commented Apr 22, 2015

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.

@h4ck3rm1k3
Copy link

@h4ck3rm1k3 h4ck3rm1k3 commented Apr 22, 2015

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.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 6, 2016

This looks duplicate of #16652 . Recommend closing this issue.

@kardianos kardianos closed this Nov 28, 2016
@golang golang locked and limited conversation to collaborators Nov 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.