Issue Details (XML | Word | Printable)

Key: DERBY-2558
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Bryan Pendleton
Reporter: Kathey Marsden
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Derby

client trhows ArrayIndexOutOfBounds exception instead of parameter out of range

Created: 17/Apr/07 08:41 PM   Updated: 17/Aug/07 10:37 PM
Return to search
Component/s: Network Client
Affects Version/s: 10.3.1.4
Fix Version/s: 10.3.1.4

Time Tracking:
Not Specified

File Attachments:
  Size
Java Source File d2558.java 2007-05-14 07:56 PM Kathey Marsden 1.0 kB
File Licensed for inclusion in ASF works IJCheckParms.diff 2007-05-09 05:06 PM Bryan Pendleton 7 kB
File Licensed for inclusion in ASF works setObjectCheckIndex.diff 2007-05-14 11:59 PM Bryan Pendleton 3 kB
File Licensed for inclusion in ASF works withJDBCTestCase.diff 2007-05-15 08:35 PM Bryan Pendleton 5 kB

Resolution Date: 16/May/07 03:00 AM


 Description  « Hide
      
The following sql gives an ArrayIndexOutOfBoundsException with client. With embedded we get the expected.
ERROR XCL13: The parameter position '2' is out of range. The number of parameters for this prepared statement is '1'.

ij> create table t (i int);
0 rows inserted/updated/deleted
ij> insert into t values (3), (4);
2 rows inserted/updated/deleted
ij> execute 'select * from t where i=?' using 'values (3,4)';
IJ WARNING: Autocommit may close using result set
JAVA ERROR: java.lang.ArrayIndexOutOfBoundsException: 1
java.lang.ArrayIndexOutOfBoundsException: 1
        at org.apache.derby.client.am.PreparedStatement.setObjectX(PreparedStatement.java:1506)
        at org.apache.derby.client.am.PreparedStatement.setObject(PreparedStatement.java:1458)
        at org.apache.derby.impl.tools.ij.util.DisplayMulti(util.java:696)
        at org.apache.derby.impl.tools.ij.utilMain.displayResult(utilMain.java:474)
        at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:351)
        at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:270)
        at org.apache.derby.impl.tools.ij.Main.go(Main.java:215)
        at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:181)
        at org.apache.derby.impl.tools.ij.Main14.main(Main14.java:56)
        at org.apache.derby.tools.ij.main(ij.java:71)

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Bryan Pendleton added a comment - 08/May/07 03:24 AM
I think it would be reasonable to address this as an IJ issue,
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.

Bryan Pendleton added a comment - 09/May/07 05:06 PM
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.

Kathey Marsden added a comment - 14/May/07 07:30 PM
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();
        

    }
}

Bryan Pendleton added a comment - 14/May/07 07:48 PM
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.

Kathey Marsden added a comment - 14/May/07 07:56 PM
Thanks Bryan, I was using the wrong setObject call.
Attached is the java repro for the issue

Bryan Pendleton added a comment - 14/May/07 08:46 PM
Thanks Kathey, your repro works for me too.

I will investigate fixing this in the client rather than in ij.

Bryan Pendleton added a comment - 14/May/07 11:59 PM
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.

Kathey Marsden added a comment - 15/May/07 01:55 AM
Patch looks good to me. I don't think we need a separate java test.

Kathey Marsden added a comment - 15/May/07 02:57 AM
actually I don't think ij2.sql runs with client, so the test would only test embedded (I think)

Bryan Pendleton added a comment - 15/May/07 03:02 AM
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.

Bryan Pendleton added a comment - 15/May/07 08:35 PM
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.


Kathey Marsden added a comment - 15/May/07 08:47 PM
looks good to me

Bryan Pendleton added a comment - 16/May/07 03:00 AM
Committed withJDBCTestCase.diff to subversion as revision 538413.

Thanks Kathey for the reviews and suggestions!