Re: binary tuple receiving patch v4

Mikko Tiihonen wrote:
>
> I'm mailing directly to you because lately my mails that I have sent to
> pgsql-jdbc(at)postgresql(dot)org have just vanished. I used to get a mail
> immediately telling that the my mail has been blocked for manual
> approval, but for the past week that has no longer happened.

The list has a maximum message size of 30k. I thought people got an
error when they sent large messages, but apparently not. I've contacted
the mail admins and hopefully they'll get it resolved. For the moment
I've put up your original patch (v4) and all additional patches
discussed below here:

http://www.ejurka.com/pgsql/patches/binary-receive/

The ones starting k4-X- are incremental diffs upon your v4 patch and
I've rolled them all up into a complete v5 version.

> Found some more time. Now I have been mainly trying to fix more of the
> corner cases and done quite a lot of testing and playing around with
> time/date formats. I hope I got them working correctly, but on the other
> hand maybe I just wrote bad test cases.
>

Well you need to at least run the existing regression tests which
revealed a number of problems (1-3 below):

k4-1 Code was not correctly setting the wasNullFlag, so after retrieving
a null value the call to ResultSet.wasNull() did not return true.

k4-2 When a query is retrieved in chunks (Statement.setFetchSize()), it
will keep executing until nothing more is returned. The test it uses
includes a check to see if it got any new Fields. Here the already
cached Fields are not new, so we don't count them.

k4-3 The code to determine whether to convert the fields to binary is
not aware of when the query is reparsed, so it can get out of sync. In
general trying to run this in AbstractJdbc2Statement is the wrong place.
Binary results are really limited to v3, so I tried to only put code
into v3/QueryExecutorImpl and v3/SimpleQuery.

k4-4 Add support for setting binaryEncoding in the regression test suite
by adjusting build.local.properties. I'm skeptical of how much testing
this actually gets us because the usage pattern that results in binary
sends is not one commonly found in the tests.

k4-5 New URL parameters need to be documented in DriverPropertyInfo.

Open issues/questions:

1) The 7.4 server does not report integer_datetimes with a
ParameterStatus message. This was only added in the 8.0 release. There
may be another way to get this info, there is the brute force, retrieve
a known value and see if it's an int or float. Disabling datetime types
for 7.4 is also an option.

2) Why doesn't AbstractJdbc2ResultSet.readDoubleValue do overflow checking?

3) Can binary timezones support infinity?

4) Why did you decide to use a BitSet instead of a HashSet to represent
whether an oid my be transferred as binary? It seems like a waste of
space unless they support a sparse representation.

5) I haven't looked at a lot of the actual conversion code, but
TimestampUtils.toJavaSecs does not make me feel warm and fuzzy. I'll
spend some more time on this later.

6) It would be good for datasources to support the binaryEncoding
parameter. I'm currently doing some work in that area, so I'll take a
look at this.

7) Is binaryEncoding a good name for the parameter? I don't want people
to confuse this with character sets which seem to give people fits already.

Kris Jurka