6 Answers
6

You always need to call next() on the ResultSet, before accessing it. If not, the pointer points to the row before the first. You do this with while-loops in your code, but some places you dont, which generates an error:

First rs is never null. If there are no rows then rs.next() will return false

ResultSet starts position BEFORE the first row. You need to call rs.next() to move to the first row which, as above, will return false if there are no rows.

The typical pattern for ResultSet use is...

while (rs.next())
{
rs.getXXX();
}

ResultSet should ALWAYS be close()d when you are done, otherwise you may leak database resources. This also applies to Connection, Statement, PreparedStatement so check your DB code in other classes.

When looping over the column metadata you need to do..

for (int i = 1; i <= numberOfColumns; i++) {
}

Note the <= rather than <. As you have correctly found that JDBC column indexes start at 1.

It would be worth considering using a library that hides all the JDBC noise. Consider spring which has a basic JDBC abstraction while retaining all the power or all the way up to hibernate which is very powerful but has quite a learning curve. In your case I'd recommend just use the spring stuff.

Also it would be good to note that when checking an object reference for null to use normal == instead of equals() (i.e. rs == null instead of rs.equals(null)). In fact, using the equals form here should also be false because of the contract on equals or throw an exception if rs really was null.
–
Kevin BrockApr 8 '10 at 2:21

Also, take a look into the javadoc/code for the DB class' execSQL(query) method. Typically, a ResultSet is never null, e.g. like when you use PreparedStatement.executeQuery. However, check into the that execSQL method to verify whether or not it will ever return a null ResultSet.

Several have pointed out some of your problems in the code. The reason for your exception is because you have not advanced to the first row using rs.next(). If you expect that your SQL statement will only retrieve a single row then you probably should change your rs.equals(null) statement to:

if (!rs.next()) {
...code here to set not found...
} else {
...code here to retrieve the columns...
}

Here are some other tips. In the code to retrieve the columns, your use of rs.getString(i) doesn't do anything, the result variable is just accumulating the column numbers with "/n" (not new-line; probably that should be "\n"). So the loop inside that section should probably become:

But concatenating to a immutable String is not good and can result in slow execution for large numbers of concatenations. Use a StringBuilder instead and initialize it to a reasonable size, something like this:

But since you need to check for the first row the next loop will not count the number of rows correctly so you will need to adjust for that, perhaps (and there is no need to check result at that point since it is guaranteed to no longer be an empty string since there must be at least one column in the table):

do {
RowCount++; // recommend using rowCount.
} while (rs.next());

In the end, what are you using result for? It seems that this is just used to determine if there were some columns in the result. If that is all then you can eliminate most of this code. For what you actually have (I don't know if this is your intent), this could be reduced to (in the else if where you do the DB interaction):

Note that if you are just after the count of rows then it is best to let the database engine do that for you - looping through the result rows just to count them also means that all that data must be sent to your application. Instead, if you just use the aggregate function then the database only needs to send a single row and column to your application.