Better support for COPY operations

[drjokepu]

The approach discussed and implemented in #181 works well and it is very useful, however, it has a number of problems:

1. It doesn’t (and maybe can’t) support COPY FROM operations.
2. It doesn’t support the binary format.
3. It doesn't support piping in data directly from an external source, data has to be provided row by row, cell by cell.
4. It reuses the existing Go SQL interface in non-trivial, confusing ways. In particular, it makes Prepare() and Exec() do things they are not meant (and documented) to do.
5. It decides whether a statement is a COPY statement or not by looking at the first four characters of the statement text and comparing it to the "COPY" word in a non case-sensitive manner. This will work most of the time but it’s not too difficult to imagine scenarios where this will fail to identify a copy operation (or maybe even identify a non-copy operation as one). The right way to do this is by parsing the query text into an AST and the right place to do that is in the backend. The front-end is simply not the right place to make decisions about the nature of queries.

In my opinion, the first three are very real shortcomings that need to be looked at, number 4 is an inconvenience the very least and number 5 is the least important as it’s only an edge case.

To address these shortcomings, I propose the following approach (which is basically an adaptation of Npgsql’s approach, something I have been using for years with much success):

• Two structs: CopyIn, CopyOut (or CopyInWriter and CopyInReader), each conforming to io.Writer and io.Reader respectively.
• Both structs maintaining a reference to the Stmt of the copy operation.
• The operation is started with copyIn.Start() or copyOut.Start()
• Then data is written to / read from these structs via the io.Writer and io.Reader interfaces or via the aid of a number of higher-level helper functions
• copyIn.End() / copyOut.End() will complete the operation.

This is a somewhat different approach than what is currently implemented but I’m convinced that it will make the support for copy operations more complete and the application code more maintainable. The downside is that it cannot be made compatible with the current approach and that it would require the application code to work with the driver directly (although that is something that’s already being done when using the existing CopyIn() and CopyInSchema() functions). A possible way to mitigate the downsides is to keep around the old approach, temporarily or permanently.

I’m keen to get this thing working (as I need a solution to problems 1 and 2) so unless there is serious opposition to this, I will start working on this immediately.

[drjokepu]

Actually now that I’ve read through the relevant code and documentation in detail, I totally understand why the current approach works the way it works and why my proposition wouldn’t work. It’s not a pretty situation.

[johto]

Actually now that I’ve read through the relevant code and documentation in detail, I totally understand why the current approach works the way it works and why my proposition wouldn’t work. It’s not a pretty situation.

Right. There's not a lot of options if we want to work on top of the database/sql API.

That said, it would be possible to add a more flexible way to COPY if we created a new connection for COPYing like the Listener I've been working on does. For some applications it could make a lot of sense to keep around a single connection for very fast/flexible COPYing (or just create one, COPY, and drop the connection), whereas some applications just don't care that much and are happy to use the interface we currently provide.

[drjokepu]

While I can see dedicated connections working for listeners, I don’t think they would offer a very good solution for copy operations. Almost every time I have used copy in operations in the past as an application developer the COPY IN statement was preceded by a CREATE TEMPORARY TABLE statement and was followed by a number of INSERT INTO ... SELECT and UPDATE statements, within the same transaction.

I think a better approach would be offering an alternative API that works with the drivers own data structures, in similar manner to how the Go driver for SQLite does it. Application developers could chose to use the sql package’s API if they need compatibility or use the pq driver-specific API if they need to have access to some more advanced features of PostgreSQL, such as COPY or LISTEN.

[johto]

While I can see dedicated connections working for listeners, I don’t think they would offer a very good solution for copy operations. Almost every time I have used copy in operations in the past as an application developer the COPY IN statement was preceded by a CREATE TEMPORARY TABLE statement and was followed by a number of INSERT INTO ... SELECT and UPDATE statements, within the same transaction.

I think that's a fair point.

I think a better approach would be offering an alternative API that works with the drivers own data structures, in similar manner to how the Go driver for SQLite does it. Application developers could chose to use the sql package’s API if they need compatibility or use the pq driver-specific API if they need to have access to some more advanced features of PostgreSQL, such as COPY or LISTEN.

I've actually been thinking about this quite a lot recently (I even built a simple proof-of-concept standalone API for an application), and I think this might be where we have to go in the future. While database/sql works great for a number of applications, it's a poor fit for others (my application couldn't deal with the pooling and retrying of connections, for example). But even if we build this new API on top of the infrastructure we expose to database/sql, there's a lot of work to do before we can even start building our extras like COPY.

Another idea I've been toying around with is being able to pull sessions from database/sql, and eventually being able to access the underlying driver connection to utilize driver-specific functionality. Though I'm not sure if this would work, or if it would even have any advantages over the standalone approach -- being able to reuse connections from the pool comes to mind, but that might just sound better than it would actually be in practice.

[drjokepu]

But even if we build this new API on top of the infrastructure we expose to database/sql, there's a lot of work to do before we can even start building our extras like COPY.

Right, it’s a lot of work indeed. I think it would be worth it though and I would be happy to contribute.

Another idea I've been toying around with is being able to pull sessions from database/sql, and eventually being able to access the underlying driver connection to utilize driver-specific functionality. Though I'm not sure if this would work, or if it would even have any advantages over the standalone approach -- being able to reuse connections from the pool comes to mind, but that might just sound better than it would actually be in practice.

What do you mean exactly by “pulling sessions from database/sql”? Do you mean somehow extracting the driver’s own data structures from structs like sql.Stmt?

[drjokepu]

being able to reuse connections from the pool comes to mind, but that might just sound better than it would actually be in practice

It’s very useful for some applications (e.g. web applications), less useful for others.

[johto]

Another idea I've been toying around with is being able to pull sessions from database/sql, and eventually being able to access the underlying driver connection to utilize driver-specific functionality. Though I'm not sure if this would work, or if it would even have any advantages over the standalone approach -- being able to reuse connections from the pool comes to mind, but that might just sound better than it would actually be in practice.

What do you mean exactly by “pulling sessions from database/sql”? Do you mean somehow extracting the driver’s own data structures from structs like sql.Stmt?

Similarly to how Begin() gives you a session that's already in a transaction, just without the "in a transaction" part. Something like:

sess := db.Session()
pqc := sess.Driver().(*pq.Conn)
pgc.CopyIn(..)
// sess could implement Begin, Exec, Query, etc.

This would be useful for at least COPY, advisory locks, and to some extent maybe LISTEN as well.

I guess you could allow accessing the underlying driver connection of a Tx as well, as you already have exclusive access to the session.

[drjokepu]

Do I understand correctly that you are proposing to use the same struct as driver.Driver and driver.Conn? It’s a very interesting idea but doesn't database/sql use only a single instance of driver.Driver for each registered driver? Wouldn't that either limit the application to a single connection or require the struct in question to manage all sessions?

[johto]

Do I understand correctly that you are proposing to use the same struct as driver.Driver and driver.Conn?

Sorry, my pseudo code in the previous post was a bit unclear.

I'm proposing that database/sql would expose the underlying driver.Conn, which the user could then type assert to a type of the underlying driver they're using. In our case, *pq.conn (which don't currently export, but would have to). The session API would save us from having to implement a standalone API (because you could Query() etc. through the session handle), and exposing the underlying driver-specific handle would allow the driver to throw in specialities of its own.

But I haven't thought this through, really. It's just an idea I've had for way too long. I started building a PoC at some point but got distracted by more pressing issues. Maybe I should try and revive that project at some point.

[drjokepu]

Yeah that makes sense, although maybe hiding the type assertion in a helper function would be a good idea; it feels like it's an implementation detail that shouldn't be exposed. Also, I'm still not really sure how the underlying driver.Conn would be exposed from sql.DB, I guess I'm missing something?

Either way, when you have a moment, why don't you push what you have started into some branch and then you know, maybe that would be something I could start to work with? I don't mean to sound pushy (sorry, if I do!) so if you won't have time do it then don't worry about it!

[johto]

(I'm on my mobile ATM, please excuse)

Just to be clear: I'm talking about changes to database/sql for Go 1.3/1.4 here. That's why the type assertion is necessary as well; database/sql can't know the type of the underlying driver connection.

In any case I'll be happy to show you where I left off if you want to pursue this approach. But this could easily coexist with the standalone API as well; in fact, I think they would complement each other quite well. It might be a better investment for you to work on the standalone API instead.