db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dyre.Tjeldv...@Sun.COM
Subject Re: [jira] Commented: (DERBY-333) Malformed if statement in org.apache.derby.impl.drda.Database.getDRDAStatement()
Date Wed, 21 Dec 2005 13:40:24 GMT
>>>>> "BP(" == Bryan Pendleton (JIRA) <derby-dev@db.apache.org> writes:

    BP(>     [ http://issues.apache.org/jira/browse/DERBY-333?page=comments#action_12360995
] 
    BP(> Bryan Pendleton commented on DERBY-333:
    BP(> ---------------------------------------

    BP(> I was interested in this bug, too, but I don't know the answer. The way that I
read
    BP(> the code, the "if" statement is trying to be an optimization, but I don't think
there
    BP(> is a need to optimize this code, for the body of the if statement is just a single
    BP(> hashtable lookup. 

I would have thought so too, but my profiler does not agree :) The point to
note here, I think, is that you can cache the result from this lookup
until you receive a different pkgnamcsn. Since pkgnamcsn is the key
used in the lookup you know that unless it has changed the statement
is the same and you can use a cached statement reference.

    BP(> In my opinion the entire getDRDAStatement() method should be re-written to read:

    BP(>   DRDAStatement newStmt = (DRDAStatement)stmtTable.get(getStmtKey(pkgnamcsn));
    BP(>   if (newStmt != null)
    BP(>   {
    BP(>     currentStatement = newStmt;
    BP(>     currentStatement.setCurrentDrdaResultSet(pkgnamcsn);
    BP(>   }
    BP(>   return newStmt;

    BP(> I think that would be simpler and more self-evident than the current code.

    BP(> The fact that simply removing the semi-colon broke the tests is interesting; it
would
    BP(> appear to mean that there are paths through the code in which the "if" statement
    BP(> evaluates to false, but either currentStatement is not actually correct, or we
need
    BP(> to call currentStatement.setCurrentDrdaResultSet(). My bet is that it's the latter:
I
    BP(> think that if you just remove the semicolon, you would break cases where the client
    BP(> is trying to switch between different result sets on the same statement.

Well, that is unfortunate. I think that we need some kind of
optimization here, but it must obviously be a correct one. It looks
like the result set is also determined by the pkgnamcsn, so it should
be possible to cache that in a similar way...

-- 
dt


Mime
View raw message