Re: Stream Copy for 8.1 - 8.3dev

On Wed, 27 Jun 2007, Kalle Hallivuori wrote:

> Stream Copy extension for 8.1 - 8.3dev drivers is available with examples at
>
> http://kato.iki.fi/sw/db/postgresql/jdbc/copy/
>

Some thoughts after giving this a read (but not a test).

1) Your CopyObjects example pays no attention to encoding or escaping. 
Calling getBytes() will give you bytes in the JVM's character set which 
can easily be different than the client_encoding which the driver will 
always set to UTF-8 (for 7.3 and up).  You also have to think about what 
happens when your data contains a null escape, delimiter, or newline 
itself.

2) I'm not sure it's helpful to have the copy methods throw both an IO and 
SQL exception.  Why not just wrap any IOExceptions inside a SQLException?

3) What is the purpose of the reuse_buffer parameter?  What is the use 
case for someone wanting a "fresh" buffer every time?

4) buildCopyQuery doesn't handle quoting/escaping of identifiers.  By 
providing something like this you're now responsible for all the "hard" 
stuff.  I would leave it out unless you're prepared to do a lot more 
thinking about it.  It also doesn't handle all the possible copy options. 
Perhaps you should split this into core copy functionality and a helper 
class that builds upon it and can provide other useful things (escaping / 
conversion of java objects to pg datatypes).

5) The coding in QueryExecutorImpl is unsafe because once you get 
CopyInResponse you loop firing away data without listening for any return 
data.  Consider a table which had a trigger on it which issued a notice 
for each row it received.  If you don't read from the server the server 
will block and then you'll block sending it data.  See the comments in 
core/v3/QueryExecutorImpl near MAX_BUFFERED_QUERIES for more details.

6) You mix warnings and errors together.  They should be kept separate.

7) When you get CopyResponses and the user hasn't provided the appropriate 
stream you bail leaving the protocol in an unknown state.  You should 
issue a CopyFail and wait for ReadyForQuery so the whole connection isn't
lost.

Kris Jurka