Redesign of database/sql

[meta keule]

Hi,

after using different pg implementations with tip and 1.0.3 and seen the different bugs and comments about

database/sql and the drivers, I have the impression, that the database/sql needs a serious redesign.

Some ideas:

1. make DB an interface. since any library using a database is supposed to support database/sql it is essential

to be able to replace the db without having to write a driver (for testing, interception et al)

2. refactor the connection pool to be a separate package and an implementation of the DB interface. Let it be customizable. 

3. It should always be clear, when a connection is open and closed, don't do magic things like autoclosing and opening connections

in the base package. connections should always be handled/controlled explicitly.

4. remove prepare statements and transactions out of database/sql. They are really not needed and not supported by all drivers and they should not,

since they could be expressed in plain SQL. More highlevel packages could provide shortcuts for them. There also should not be opinionated linkage

between connection handling and transactions/prepare statements.

5. instead make a typed struct (ConnOptions or so) for database connections instead of fragile connection strings. let the driver transform the connection struct to whatever it needs. A port should be an int, a password a string etc. provide a way to transform an Url to this kind of ConnOptions struct.

Regards,

Benny

[David Symonds]

A lot of what you suggest cannot be done without violating our compatibility promise, and would break existing code.

[meta keule]

Does the compatibility promise cover the standard libraries/packages? 

I thought it would just cover the language.

I think there are changes for 1.1 in the standard libraries/packages that will break code as well.

Correct me if I am wrong.

Any way, it is not rewritten overnight, so it is too late for 1.1, but should be considered for 1.2 or 2.

A external package could be provided for backwards compatibility.

Anyway I am tired of the argument, that improvements would break existing code.

That way you could not go anywhere.

Nowhere I said, it must go into 1.1, but seriously the database/sql package is broken anyway (by design)...

[Julien Schmidt]

What about a fork of database/sql?
Except of 5) all the proposed changes can be done while still using drivers implementing the database/sql/driver interface.

[meta keule]

Am Freitag, 26. April 2013 09:27:52 UTC+2 schrieb meta keule:

I think there are changes for 1.1 in the standard libraries/packages that will break code as well.

Correct me if I am wrong.

such as

The previous implementation of ListenUnixgram returned a UDPConn as a representation of the connection endpoint. The Go 1.1 implementation instead returns a UnixConn to allow reading and writing with its ReadFrom and WriteTo methods.

 

[Andrew Gerrand]

On 26 April 2013 09:27, meta keule <marcre...@googlemail.com> wrote:

Does the compatibility promise cover the standard libraries/packages? 

I thought it would just cover the language.

I think there are changes for 1.1 in the standard libraries/packages that will break code as well.

Correct me if I am wrong.

You're wrong.

http://golang.org/doc/go1compat.html

Andrew

[Andrew Gerrand]

On 26 April 2013 09:27, meta keule <marcre...@googlemail.com> wrote:

Anyway I am tired of the argument, that improvements would break existing code.

That way you could not go anywhere.

That's not the argument. The argument is that backward incompatible changes break existing code, and IMO it's a pretty hard one to refute.

If you want a redesigned database/sql package, it'll need to be somewhere else, OR it'll need to be in the form of backward compatible changes to the existing package.

Andrew

[Rémy Oudompheng]

The exceptions are only for obviously broken signatures, which is the case here.
A redesign should create a new package or additional types/functions, not modify an existing one.

Rémy.

[meta keule]

One can argue, if we need database/sql  in the standard library at all.

The only purpose I can see, is unifying database drivers.

At such it fails, since it makes lots of assumptions that it should

not make and makes some weird uncontrollable and undocumented

connection pooling.

Making a driver that needs to fullfill this strange interface only to 

prevent the work, database/sql  tries to do and to build another

interface for driver developers makes no sense.

It is also a communication issue. Someone starting to write

a driver would likely not use the new interface, but the old one.

[Andrew Gerrand]

On 26 April 2013 09:42, meta keule <marcre...@googlemail.com> wrote:

One can argue, if we need database/sql  in the standard library at all.

The only purpose I can see, is unifying database drivers.

It's there, we can't remove it. Did you read the document I linked? There's no point arguing about this.

Andrew

[meta keule]

Anyway, there should be some consensus, how the new library should

be build, even if it is developed outside the standard library.

If the new library is ready, it should be recommended in the documentation.

Now we can start discuss and build a consensus about the new library, regardless

of how it may be integrated in to the project.

[Andrew Gerrand]

On 26 April 2013 09:45, meta keule <marcre...@googlemail.com> wrote:

Anyway, there should be some consensus, how the new library should

be build, even if it is developed outside the standard library.

I suggest you start by drafting an API, then see if anyone is interested in working with you.

Andrew

[meta keule]

Ok, lets see.

You run a software project. Now you have 2 problems:

- management

- development

You are talking about management.

I am talking about development.

If a better replacement is developed, we could discuss, what would be the best way

to manage updates or what.

You should not prevent development by saying it's unmanageable.

This way, no development would happen.

[meta keule]

We have existing drivers and people that have written them.

They are different, the databases are different and the drivers.

We should build a consensus as community by considering

the experiences of the writers of the drivers and of the users,

it makes no sense, if a person makes such decisions on his own.

I made some pointers to the direction, I think, things should change

and would love to hear technical arguments from database/sql authors,

database driver authors and users, not discuss compatibility issues.

[Taru Karttunen]

On 25.04 23:22, meta keule wrote:
> 1. make DB an interface. since any library using a database is supposed to
> support database/sql it is essential
> to be able to replace the db without having to write a driver (for testing,
> interception et al)

Why not better testing in the database/sql tests?

> 2. refactor the connection pool to be a separate package and an
> implementation of the DB interface. Let it be customizable.

This could probably be done without breaking database/sql API.

> 3. It should always be clear, when a connection is open and closed, don't
> do magic things like autoclosing and opening connections
> in the base package. connections should always be handled/controlled
> explicitly.

Many of the users of the base package *want* this.

> 4. remove prepare statements and transactions out of database/sql. They are
> really not needed and not supported by all drivers and they should not,
> since they could be expressed in plain SQL. More highlevel packages could
> provide shortcuts for them. There also should not be opinionated linkage
> between connection handling and transactions/prepare statements.

This would make database/sql unusable for many applications.

> 5. instead make a typed struct (ConnOptions or so) for database connections
> instead of fragile connection strings. let the driver transform the
> connection struct to whatever it needs. A port should be an int, a password
> a string etc. provide a way to transform an Url to this kind of ConnOptions
> struct.

I'd consider this a step backwards. Databases may not even have a
concept of an port or the "port" may be a filepath, or a series
of ints.

- Taru Karttunen

[Kamil Kisiel]

Or the connection string could be a list of servers, or have a myriad of non-standard options that would be shoved in to an Options string field. Then we'd be back to where we are.

[Julien Schmidt]

Why do you even need a new driver interface?
I see no reason (except 5. ) why the package you described wouldn't work with the current database/sql/driver/ interface, which is very minimal and extended by optional interfaces.

[Julien Schmidt]

And 5. could be done via an optional interface.

[meta keule]

The deeper I dig into the sql package, the more rotten it looks to me.
connections should be decoupled from prepare statements and transactions.
Drivers should not be forced to fullfil an interface with prepare statements and transactions if
they do not support it. That could be optional interfaces and there could be abstractions for this optional interfaces. It is even not documented, what the driver is supposed to do, if it does not
support Begin(). Should it behave, as if it could do transactions but just exec the sql? Should it
always return an error? I could not find a single postgres driver, that handled transactions properly (via the interface). But it was no problem to create them within Sql.
Should I do the Sql transaction then? Or circumvents it the connection pooling logic?
Btw prepare statements dont work either.

The whole package is broken and drivers don't work with them. Lots of implicit assumptions.
Then this stupid Next(). It must be called before every Scan() and its only purpose is to prepare a Scan(). Then why just make it part of Scan() and ditch Next() and let Scan() return io.EOF when there are no more rows.

Then this strange separation of Exec() and Query().

The most basic thing, such a package could provide, is a reliable and controllable standard way to make connections and send queries and get results. And it fails to do those basic things.

Instead it makes lots of assumptions that do not hold for some databases.
If you forget to call Next() you're stuck. If you make transactions around your queries it may work
or not. If you use goroutines (which are automagically created below your feet if you use net/http
BTW) you suddenly might need to reconsider your reusage of *DB.

Also is it completely unclear what *DB is:
- it is no driver
- it is no connection
- it is no database
- it is no interface

and you can't get rid of when using 3rd party libraries that need a database, because it is no interface. Then you have to write a "driver" to replace it. How ridiculous is that?

The source code of database/sql is full of comments like this:

 // TODO(bradfitz): possibly remove the restriction above, if
 // enough driver authors object and find it complicates their
 // code too much. The sql package could be smarter about
 // refcounting the statement and closing it at the appropriate
 // time.

// But the TODO is that
// for a lot of drivers, this copy will be unnecessary.  We
// should provide an optional interface for drivers to
// implement to say, "don't worry, the []bytes that I return
// from Next will not be modified again." (for instance, if
// they were obtained from the network anyway) But for now we
// don't care.

// If the underlying database driver has the concept of a connection
// and per-connection session state, the sql package manages creating
// and freeing connections automatically, including maintaining a free
// pool of idle connections. If observing session state is required,
// either do not share a *DB between multiple concurrent goroutines or
// create and observe all state only within a transaction.

lots of ifs and whens:
$ grep if sql.go | wc
142     895    4415

all issues for database/sql: 50
all issues for net/smtp: 21

and database/sql does nothing but providing an interface for database drivers.

database/sql is in a really bad shape and that is because it tries to do too many
things, makes too many assumptions and does not get the basics right.

[meta keule]

I was not aware of that they are that different. But if they are, why not make Open() part
of the Conn interface and let the drivers handle the connection options on their own in a type
save way?

Also no need to make a string for driver - what a stupid idea.
Why not

Open(drv Driver) (*DB, error)

and

type Driver interface {
    // Open returns a new connection to the database.
    // the parameters to the connection are handled by the driver
    // The returned connection is only used by one goroutine at a
    // time.
    Open() (Conn, error)
}

and let the driver authors do something like

var driver = .... blah (set it in init)

type New struct {
   User string
   Password string
   ...
}

func (ø New) Open() (Conn, error) {
  driver.Open(ø.User, ø.Password,...)
}

then the user could simply call

db := sql.Open(pg.New{User: "blah",Password: "blah"})

[meta keule]

Am Freitag, 26. April 2013 11:05:18 UTC+2 schrieb Taru Karttunen:

On 25.04 23:22, meta keule wrote:
> 1. make DB an interface. since any library using a database is supposed to
> support database/sql it is essential
> to be able to replace the db without having to write a driver (for testing,
> interception et al)

Why not better testing in the database/sql tests?

how does better testing in the database/sql tests enable us to mock out a database
or to intercept calls or to replace sql.DB with something better...

> 2. refactor the connection pool to be a separate package and an
> implementation of the DB interface. Let it be customizable.

This could probably be done without breaking database/sql API.

without breaking the API, but not without breaking the behaviour.
anyway, it should be done, and it seems someone recently tried to make it happen:
https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/JFe0QD7PtDU

 

> 3. It should always be clear, when a connection is open and closed, don't
> do magic things like autoclosing and opening connections
> in the base package. connections should always be handled/controlled
> explicitly.

Many of the users of the base package *want* this.

What do they want? A broken package that connects and disconnects in an uncontrollable way?
It *could* be provided as an optional bonus. An if sql.DB was an interface the connection pool
or what ever preferred magic connection maker could fullfill the interface and be used by the users
that want it. But it should be possible to resort to a basic functionality that allows explicit handling of connections.
 

> 4. remove prepare statements and transactions out of database/sql. They are
> really not needed and not supported by all drivers and they should not,
> since they could be expressed in plain SQL. More highlevel packages could
> provide shortcuts for them. There also should not be opinionated linkage
> between connection handling and transactions/prepare statements.

This would make database/sql unusable for many applications.

No, it is not. There is no need to make support of transaction/prepare statements a requirement for a driver interface.
It could be an optional interface however. Linkage could optionally be provided by a connection pool
fullfilling a sql.DB interface.
 

> 5. instead make a typed struct (ConnOptions or so) for database connections
> instead of fragile connection strings. let the driver transform the
> connection struct to whatever it needs. A port should be an int, a password
> a string etc. provide a way to transform an Url to this kind of ConnOptions
> struct.

I'd consider this a step backwards. Databases may not even have a
concept of an port or the "port" may be a filepath, or a series
of ints.

see my other post to the topic. if they are so different the connection parameter should be handled by the drivers
in a typesafe way.

- Taru Karttunen

[meta keule]

What about this:

We could even do better without changing database/sql.

Why not make something directly in the "database" package (top level path)
that only provides a basic interface for driver authors regardless of whether they
are Sql databases or not with the following features:

- make connections
- close connections
- send bytes to the server and get the response back


no driver interface, but a Database interface

type Database interface {
   Open() (error) // opens a connection 
   Close() (error) // closes a connection
   Query([]byte) (Response, error) // queries
}

type Response interface {
   Scan(values ...interface{}) error // no "Next()" required, if there are no values to get, return io.EOF
   Message() string // for messages like affected rows etc.
}

let the driver authors implement all of them including scan and the support of different types.
provide helpers for the driver authors for converting basic types and let them do the rest (e.g. decide what types are supported etc)

Document that they are not supposed to create any goroutines in the driver, nor to do caching of queries or connection pooling.

then database/pool could provide some pooling, fullfilling the Database interface, database/cache could do some caching
and database/sql could behave as now, making use of the others.

Regards,
Benny

[Rémy Oudompheng]

On 27 Apr 2013 08:11, "meta keule" <marcre...@googlemail.com> wrote:
>
> The deeper I dig into the sql package, the more rotten it looks to me.
> connections should be decoupled from prepare statements and transactions.

What about systems that prepare statements on server side?
A prepared statement is not just "a string with placeholders".

Remy.

[meta keule]

What can't be done by a highlevel database/sql/prepare package that handles
prepare statements for dbms that fullfil an optional Preparer interface?
That could do whatever is felt appropriate with the connections or make use of a pool.
But it should be optional. That was my point.
 

[Andy Balholm]

Most of the peculiarities of the database/sql package appear to be results of the attempt to provide connection pooling that is transparent to both clients and drivers. For example, the reason it uses an explicit Begin method returning a transaction object instead of just db.Exec("BEGIN") is that all statements in a transaction need to be on the same database connection.

Yes, the abstraction is leaky, but I like being able to have a global db object that I can call methods on from multiple goroutines simultaneously. Instead of explicitly fetching a connection from the pool, using it, and returning it to the pool, I can just call db.Exec and let the sql package take care of the rest.

The design is not perfect, but I think that overall Brad came up with a good interface. The reason there are so many issues is that it is an innovative design that tries to hide a lot of complexity.

[Dougx]

This.

For what its worth I'm strongly in favour of the fluent nhibernate style of chained SQL statements.
(eg. Stmt().Select().From("TableBlah").Where("field > 10").Limit(10), Table().AddColumn("field").Int32().AddColumn("xxx").DateTime().

The api is well documented and well liked my many many people.

It's also database independent, as proved by the large number of supported targets for nhibernate and could be written as a set of driver either 1) directly connecting to the database, or 2) building on top of the existing sql package.

I certainly wouldn't advocate it as a *modification* for the database/sql package; it would either stand along side, or completely replace that (my preference is, as I've made abundantly clear previously, that database/sql should just go away entirely, but certainly you could create database/fluent as an alternative and let natural no-one-is-using-database/sql-because-it-sucks take it's course)

~
Doug.

[Dougx]

Just as a dirty hack, proofing of concepting this idea for pq / mysql / sqlite:
github.com/shadowmint/go-fluent

Notice how all the drivers use the src/fluent/drivers/tests as a common test suite to run the same tests using the identical code on different drivers.
Isn't that a neat trick?

~
Doug.

[Daniel Theophanes]

On Sunday, April 28, 2013 7:03:20 AM UTC-7, Dougx wrote:

For what its worth I'm strongly in favour of the fluent nhibernate style of chained SQL statements.
(eg. Stmt().Select().From("TableBlah").Where("field > 10").Limit(10), Table().AddColumn("field").Int32().AddColumn("xxx").DateTime().

If you do the above, you are giving up the power SQL gives you. I don't think you could express an upsert, recursive CTE, or even basic "is null" or sub-queries or left joins... Just, write in SQL. It's easy. I do think SQL and rdbms need work to fit architecture, but I think this is a net loss, not a net gain.

-Daniel

[Nirbhay Choubey]

Hello Benny,

On Fri, Apr 26, 2013 at 11:52 AM, meta keule <marcre...@googlemail.com> wrote:

..cut..

5. instead make a typed struct (ConnOptions or so) for database connections instead of fragile connection strings. let the driver transform the connection struct to whatever it needs. A port should be an int, a password a string etc. provide a way to transform an Url to this kind of ConnOptions struct.

I found dataSourceName (a string) quite intuitive though. IMHO this gives the driver writers
freedom to define their own connection-string and parse it accordingly inside the driver. For
example, lets think of a driver that provides an option of connecting over SSL. Now, thinking
more like jdbc's connection URL, one can supply all the required SSL options through the
dataSourceName: " ... useSSL=true&sslCert=some-cert.pem&sslKey=some-key.pem ... ".

You can think of many such unforeseen driver-specific options that dataSourceName can
support if left as string.

My few cents.

-- Nirbhay

Regards,

Benny

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Aaron France]

Then you could just take an interface Options parameter. Seems cleaner than encoding stuff into a string.

Aaron