Re: Bug with caching SQLTypes in Connection:getSQLType(oid)

OK, I finally got a few hours to look into this and I agree with Ned's
findings. It is just a one line fix, which I have committed and built
new jar files and placed up on the website.

--Barry

Ned Wolpert wrote:

> I included a pretty chessy test case for this in the email. You'll have
> to watch the log output with debug_query set to true in the postgresql
> conf file. Look for the number of times the query is executed. With
> the current cvs version, the query "select typname from pg_type where
> oid = 1043" occurs four times. With my fix, it occurs once.
> (args are url, username and password)
>
>
> On Mon, 2001-12-10 at 12:43, Dave Cramer wrote:
>
>>If we are going to try to get this in as a bug fix for 7.2RC1 then I
>>would like to see something more conclusive.
>>
>>Can we verify that the patch works, or fixes something?
>>
>>Dave
>>
>>-----Original Message-----
>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Ned Wolpert
>>Sent: Sunday, December 09, 2001 4:49 PM
>>To: pgsql-jdbc(at)postgresql(dot)org
>>Subject: Re: [JDBC] Bug with caching SQLTypes in
>>Connection:getSQLType(oid)
>>
>>
>>--- Antonio Fiol Bonnín <fiol(at)w3ping(dot)com> wrote:
>>
>>>Well, It is *certain* that the existing code is somehow broken. And it
>>>is visible even at first sight. How to correct it is probably harder,
>>>
>>Yes, I noticed that too. The 'quick/easy' fix that I'm using is to
>>update sqlTypeCache hashtable which mapped 'oid -> SQLType'. That was
>>the patch that I sent in. It doesn't break anything 'more' since a) if
>>its not in cache it puts the value in the sqlTypeCache
>>hashtable to begin with and b) if it is found in the check, then a
>>potentially bogus result is returned. (Odds are it won't, since it
>>currently checks typeOidCache, which maps 'PGType -> oid' instead. I
>>doubt one would ever have an oid they send into the function, which
>>would then check the typeOidCache. The oid key wouldn't match the
>>PGType key to find the oid value. Ugh... my brain hurts... ;-)
>>
>>
>>>but if you look into the following non abstract method,
>>>getOID(String),
>>>you will see that it is looking up in the same hashtable, but using a
>>>different class.
>>>
>>Yes. in getOID(String), it uses the typeOidCache correctly. (If the
>>comments defining the variable typeOidCache are correct.) It maps the
>>string of the PGType -> the Integer oid.
>>
>>
>>>In getSQLType(int), it is looking up an entry for an
>>>Integer. In getOID(String), it is looking up in the same hashtable,
>>>
>>but
>>
>>>using a String. This is obviously wrong or a very bad programming
>>>practice. (I vote wrong).
>>>
>>Yeah, I think it was just a mistake made that didn't give incorrect
>>results since the item is never cached. (Or, can never be found.)
>>
>>
>>>I can't go much deeper by now. Sorry.
>>>
>>Thanks, you've given me the info I need on the original discussion. I
>>think we should see if we can put this fix in before 7.2 is released.
>>Perchance in the 7.2rc1? Folks? Can someone else verify that our
>>analysis is correct before the rc1 release?
>>
>>=====
>>Virtually, | "Must you shout too?"
>>Ned Wolpert | -Dante
>>wolpert(at)yahoo(dot)com |
>>_________________/ "Who watches the watchmen?"
>>4e75 -Juvenal, 120 AD
>>
>>-- Place your commercial here -- fnord
>>
>>__________________________________________________
>>Do You Yahoo!?
>>Send your FREE holiday greetings online! http://greetings.yahoo.com
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 6: Have you searched our list archives?
>>
>>http://archives.postgresql.org
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 2: you can get off all lists at once with the unregister command
>> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>>
>>
>>------------------------------------------------------------------------
>>
>>import java.sql.*;
>>import org.postgresql.Connection;
>>
>>public class PostgreSQLTest {
>>
>> public void runTest (String []str) throws Exception {
>> String url = str[0];
>> String user = str[1];
>> String password = str[2];
>>
>> Class.forName("org.postgresql.Driver").newInstance();
>> org.postgresql.Connection c = (org.postgresql.Connection) DriverManager.getConnection(url, user,password);
>> // look up a void oid
>> for ( int i=0;i<5;i++) {
>> c.getSQLType(1043);
>> }
>> }
>>
>> public static void main(String[] args) {
>> try {
>> PostgreSQLTest test = new PostgreSQLTest();
>> test.runTest(args);
>> } catch (Exception e) {
>> e.printStackTrace();
>> } // end of try-catch
>> } // end of main()
>>}
>>