>>>>> "BP(" == Bryan Pendleton (JIRA) <[email protected]> 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

Reply via email to