db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Deepa Remesh" <drem...@gmail.com>
Subject Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
Date Mon, 27 Feb 2006 17:14:19 GMT
Thanks Bryan for reading my patch. Please see my answers below:

On 2/26/06, Bryan Pendleton <bpendleton@amberpoint.com> wrote:

> Three small comments:
>
> 1) It wasn't clear to me why these needed to be applied as two
> separate patches. I would be comfortable putting the test and the
> fix in the same commit; is there something I'm missing here?

The patches can be committed together. It is just that patch2 (tests)
should not be committed before patch1 (code changes). I'll mention
this when I upload the next version of patch.

>
> 2) There's a mysterious comment in DRDAStatement.initialize():
>
>    +     * TODO: Need to see what exactly it means to initialize the default
>    +     * statement. (DERBY-1002)
>
> Does this mean that there is still a loose end to tie up here?
>

Yes. The comments in the code where DRDAStatement.initialize() is
being called is confusing. They say "// initialize statement for
reuse". I think this is confusing because we have reset() method which
also is called to re-use statements stored in stmtTable. initialize()
is called for default statement, which is used differently. From what
I understand, there is just one default statement per database and
default statement is different in the way it gets initialized and
used. e.g: stmt variable used in default statement is created only
once in Database.makeConnection. So, we cannot call reset() for
default statement. I think it would be good to understand this part
and fix up the comments/code so that it is clear. Kathey and Knut also
mentioned that the use of initialize method at few places was
confusing to them. Sorry, I did not mention explicitly that the patch
has more to-do items  in my latest comment.

> 3) It took me a little while to realize that the idea of testStatementReuse
> is that it sets up the section tables in such a way as to provoke statement
> re-use in tests jira_491_492 and testImplicitClose.

[snip ... very good suggestions to improve comments in the test case]

Thanks for the suggestions. I'll fix the comments and upload a new
version of patch2.

Thanks,
Deepa

Mime
View raw message