|
Attached is 'ijCheckParms.diff', a patch proposal.
This patch modifies IJ so that it cross-checks the number of parameters in the prepared statement against the number of columns in the provided result set, and complains if the two do not have the same size. The patch also includes a few tests. derbyall and suites.All pass. Please have a look at the patch proposal and tell me what you think. It seems to me that since the ArrayIndexOutOfBounds exception is being thrown from within client, we should be able to get a java reproduction for this outside of ij. I tried, but got a perfectly reasonable error message with the code below, so I am not sure how to reproduce, but it does seem like just working around the issue in ij is not the best solution.
import java.sql.DriverManager; import java.sql.SQLException; import java.sql.Statement; import java.sql.Connection; import java.sql.ResultSet; import java.sql.PreparedStatement; public class d2558 { public static void main(String[] args) throws Exception { Class.forName("org.apache.derby.jdbc.ClientDriver"); Connection conn = DriverManager.getConnection("jdbc:derby://localhost:1527/wombat;create=true;traceFile=trace.out"); Statement s = conn.createStatement(); try { s.executeUpdate("DROP TABLE t "); } catch (SQLException se) {} s.executeUpdate("Create table t (i int)"); s.executeUpdate("insert into t values (3), (4)"); PreparedStatement ps = conn.prepareStatement("select * from t where i=?"); ps.setObject(1,new Integer(3)); ps.setObject(2,new Integer(4)); ResultSet rs = ps.executeQuery(); } } Hi Kathey, thanks for the feedback!
I see what you mean. I also notice that IJ's code is calling the 4-argument version of setObject, not the 2-argument version, and I wonder if that is why your repro didn't provoke the error. I will look more closely at the client code, using your repro as a basis. Thanks Bryan, I was using the wrong setObject call.
Attached is the java repro for the issue Thanks Kathey, your repro works for me too.
I will investigate fixing this in the client rather than in ij. Attached is setObjectCheckIndex.diff, an alternate patch proposal.
Instead of checking the parameter index in IJ, this change checks the parameterIndex at the start of the client's setObject() method, using the same method as is done for the other overloads of setObject. As Kathey observed, it's better to fix this in the client code directly because then it fixes not only the IJ case but also all other code that might use the client APIs, such as a user's application. The patch proposal contains a new test case, as an IJ script, taken from the initial repro. I'd appreciate feedback about whether we should also add some test cases which directly call the JDBC API as well, based on Kathey's d2558.java test program, or whether that would be overkill and it's adequate just to test this via IJ. derbyall and suites.All testing was clean with this patch. Patch looks good to me. I don't think we need a separate java test.
actually I don't think ij2.sql runs with client, so the test would only test embedded (I think)
I'll look into adding your repro as a test case in something like
derbynet/PrepareStatementTest.java. It should be pretty straightforward, and that way we're sure we have a test case covering client code. Attached is 'withJDBCTestCase.diff', an updated patch proposal
which contains a new test in the PrepareStatementTest suite. I left the ij2.sql test in place, because I think it is a reasonable test and it doesn't add much time and eventually we might make ij2.sql run against the client, in which case this test case will also help verify this change. I checked that PrepareStatementTest fails as expected in client mode without the code fix, and works in both client and embedded with the code fix, and derbyall and suites.All were all clean. Committed withJDBCTestCase.diff to subversion as revision 538413.
Thanks Kathey for the reviews and suggestions! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
by enhancing util.DisplayMulti() to compare ResultSetMetaData.getColumnCount()
against ParameterMetadata.getParameterCount(), and complain
if the two counts do not match.
At least, it seems to improve this particular test case. I'll try to work it
up as a proper patch, and see what results I get.