>>>>> "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