RE: [INTERFACES] Bugs in 7.1beta JDBC driver

> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: Thursday, December 21, 2000 10:18 PM
> To: pgsql-interfaces(at)postgresql(dot)org
> Subject: [INTERFACES] Bugs in 7.1beta JDBC driver
>
>
>
> I recently pulled and built the 7.1beta1 code and ran our regression
> tests against this version. Our app is java based and uses the JDBC
> drivers for client access. Almost all of our regression
> tests failed.
> In looking into this further there are some significant problems with
> the latest JDBC code.
>
> Specifically the code that has been added to provide caching
> of byte[]'s
> doesn't work correctly. This code makes some assumptions that are not
> valid, specifically it assumes that when a new SQL statement is
> executed, all previously used byte[]'s are no longer in use and can be
> reused. Specifically the pg_stream.deallocate() call that appears in
> the Connection.ExecSQL() method:
>
> // Deallocate all resources in the stream associated
> // with a previous request.
> // This will let the driver reuse byte arrays that
> has already
> // been allocated instead of allocating new ones in order
> // to gain performance improvements.
> pg_stream.deallocate();
>
> The assumption here is invalid. For example if code does the
> following:
>
> 1) execute a query A
> 2) for each row of the result set execute a different query B
>
> When the query B is executed for the first row of query A, the
> deallocate() method gets called which causes *all* byte[]'s
> originating
> from the pg_stream object (which includes the row data in the
> ResultSet
> object) to be reclaimed and reused, including those
> associated with the
> subsequent rows of query A.
>
> So what ends up happening is that the data in query A's result set
> starts to get corrupted as the underlying byte[]'s get reused by
> subsequent queries.
>
> Believe me this causes all sorts of problems.

Yes, this is causing all sorts of chaos with DatabaseMetaData as well.

[snip]

> Finally, my app does a lot of queries that select values that
> are > 256
> bytes long. The current cache implementation doesn't help in these
> cases at all (as it only caches arrays up to 256 bytes), but
> it is with
> these large byte[]'s where the overhead of allocating and then garbage
> collecting them is greatest and could most take advantage caching
> functionality.
>
> I have looked at the source code and thought about how to fix
> the above
> problems, and I have not been able to come up with anything that works
> well. While is certainly would be possible to code something that
> solves these problems, I fear that the result would actually be slower
> than the current situation in 7.0 where no cache exists and lots of
> arrays are allocated and then reclaimed by the garbage collector.
>
> I would suggest that the cache code be pulled out for
> 7.1beta2, until a better overall solution is found.

As I posted earlier it's going to be Wednesday (27th) before I can do
anything. I was going to dig in deep to see what was causing the weirdness
in DatabaseMetaData.getTables() but you seem to have found it. Gunnar wrote
the cache and I'm cc'ing him to see if he knows of anything that's been
missed out.

So, on Wednesday I'll attempt to remove the cache. We do need some caching
but we have to keep it thread & Statement safe. Perhaps we need to have the
query cache linked to Statement (which under the specs can have only one
ResultSet) rather than the Connection/PG_Stream. Internal queries must also
be handled some how.

Peter (ps: do not reply direct as this address is going soon)

--
Peter Mount
Enterprise Support Officer, Maidstone Borough Council
Email: petermount(at)maidstone(dot)gov(dot)uk
WWW: http://www.maidstone.gov.uk
All views expressed within this email are not the views of Maidstone Borough
Council