Hi Deepa, Thanks for pulling this patch together. This is excellent work, and I think the new structure of initialize/close/reset/setTypDefValues is much cleaner and easier to read.
I successfully applied your patches, and I can confirm that your new regression test provokes the expected failure(s) without the patch, and succeeds with the patch. 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? 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? 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. That is, testStatementReuse isn't directly testing the problem, but rather is setting things up so that the other two tests will see a "more interesting" environment. It seems like it would be nice to be more explicit about this technique in the comments at least, because it's kind of subtle and when I first read the new test code, I thought that you had omitted something because the end of testStatementReuse() seems to trail off without testing anything; moreover, it refers to something called cSt2 which doesn't actually exist in that test case: + // Close cSt1 so that cSt2 will reuse same DRDAStatement + cSt1.close(); My suggestion is: - fix that last comment in testStatementReuse to make it clear that we are arranging to have a re-usable statement be "ready" at the end of this method - fix the method-level comments for testStatementReuse so that instead of saying: + * This test creates statements and closes them to provoke the network server to + * re-use statements to test that re-use happens correctly. It gives error without + * patch for DERBY-1002. they instead make it clear that this test sets up a statement which is ready to be re-used, so that the *next* test will start by reusing an old statement. - add a one-line comment at the two places (in testImplictClose() and in jira_491_492()) where you have planted the call to testStatementReuse() saying something like: // Cause an existing statement to be opened and closed so that it will be re-used Hope this is helpful, bryan
