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@postgresql.org
>>[mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Ned Wolpert
>>Sent: Sunday, December 09, 2001 4:49 PM
>>To: pgsql-jdbc@postgresql.org
>>Subject: Re: [JDBC] Bug with caching SQLTypes in
>>Connection:getSQLType(oid)
>>
>>
>>--- Antonio Fiol Bonnín <fiol@w3ping.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@yahoo.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@postgresql.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()
>>}
>>