Re: Binary protocol patch v7

On Sun, 7 Jan 2007, Mikko Tiihonen wrote:

> Just to remind you that the I updated the patches for sending receiving
> columns in v3 binary format are available at:
> http://mokki.dyndns.org/~mtiihone/postgresql/binarytransfer/
>
> Also, Kris, could you please try to find time to review the patch again
> and see what still needs to be fixed/clarified/tested before the patch
> can be accepted.
>

I was actually looking at the v7 patch earlier this week, specifically
looking at the conversion code. To try and throw some of the toughest
test cases at it I modified the timestamp and time zone test code like
k1-timestamp-test.patch. When running the tests with a
build.local.properties including binaryTransfer=true and
preparethreshold=1 this test fails.

So I thought about ways to get more testing without having to rewrite
every single regression test. I'm not suggesting committing any of this
code, but it's just something I hacked together to try more cases.

k2-describe-then-execute.patch issues a describe then an execute for every
prepared statement so it won't need to be executed more than once to start
using the binary transfer mode if we've set prepareThreshold to 1. This
caused only one failure in an updatable resultset test because the results
of ResultSet.refreshRow() were in binary while the ResultSet was in text.
This failure doesn't concern me because without the hack the
PreparedStatement is only used once and won't trigger a binary receive.

So then I tried to k3-stmt-as-prepstmt.patch to send all
Statement.executeQuery calls using a PreparedStatement to trigger binary
receiver. This resulted in a deadlock in the regression tests that I did
not have time to investigate.

Even though this didn't work, I think it's the way to go to convince
people that the new conversion code works exactly as the existing code
does. I don't care how ugly it is or how slow it is, if there's a way to
get the majority of our existing tests to run through the binary codepath
I'd be happy to apply the patch.

Kris Jurka