Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty result set throws nullpointer for querySingle #54

Closed
bhowell2 opened this issue Nov 10, 2017 · 3 comments
Closed

Empty result set throws nullpointer for querySingle #54

bhowell2 opened this issue Nov 10, 2017 · 3 comments
Assignees
Labels
Milestone

Comments

@bhowell2
Copy link

Version

3.5.0

Context

A query that returns an empty resultset (with querySingle and querySingleWithParams) will cause a NullPointerException.

Here's the code in SQLOperations.java:

  default SQLOperations querySingle(String sql, Handler<AsyncResult<JsonArray>> handler) {
    return query(sql, execute -> {
      if (execute.failed()) {
        handler.handle(Future.failedFuture(execute.cause()));
      } else {
        final ResultSet rs = execute.result();
        if (rs == null) {
          handler.handle(Future.succeededFuture());
        } else {

// NOTE HERE: 
// The result set may not be null, but may be empty. The call to results.get(0) will cause an NPE.
          List<JsonArray> results = rs.getResults();
          if (results == null) {
            handler.handle(Future.succeededFuture());
          } else {
            handler.handle(Future.succeededFuture(results.get(0)));
          }
        }
      }
    });
  }

This fix may be simple in that all that needs to be checked is adding:
if (results == null || results.size() == 0) (and then return Future.succeededFuture()), or this may be a bit deeper of an issue in that ResultSet is not expected to return an empty list if there were no results in the database for the query? (I'd imagine this is okay, but checking here to make sure because of the way this was written.) Future.succeededFuture() is going to return a null result, is this desirable?

@leokongwq
Copy link

Why the bug has not been fixed?

@bhowell2
Copy link
Author

bhowell2 commented Dec 1, 2017

Probably because I didn't post it to vertx-dev google groups. Posted it there now. The fix is pretty easy, but I wasn't sure which way they wanted to take it.

@pmlopes
Copy link
Member

pmlopes commented Dec 1, 2017

Fixed by aa72de8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants