Remove DML check on getUpdateCount()

[SimbaGithub]

Overview

The current behavior for getMoreResults() returns false for every DDL statement in a multistatement query, even though there can be more results accessible later in the multistatement.

External contributors - please answer these questions before submitting a pull request. Thanks!

Please answer these questions before submitting your pull requests. Thanks!

1. What GitHub issue is this PR adressing? Make sure that there is an accompanying issue to your PR.

   Fixes #571

2. Fill out the following pre-review checklist:

   • [ ] I am adding a new automated test(s) to verify correctness of my new code
   • [ ] I am adding new logging messages
   • [ ] I am modyfying authorization mechanisms
   • [ ] I am adding new credentials
   • [ ] I am modyfying OCSP code
   • [ ] I am adding a new dependency

3. Please describe how your code solves the related issue.

Remove the DML check on getUpdateCount() so the check is regardsless of DML/DDL.

Pre-review checklist

• [ ] This change has passed precommit
• [ ] I have reviewed code coverage report for my PR in (Sonarqube)

[JpnTrzdTzURv3lOIQad0]

...but getMoreResults() should return true whenever there are more statements available for iterating in the multistatement

getMoreResults() Returns true if the next result is a ResultSet object; false if it is an update count or there are no more results

[sonarqubemergegate[bot]]

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[sfc-gh-ext-simba-lb]

uld return true whenever there are more statements available for iterating in the multistatement

...but getMoreResults() should return true whenever there are more statements available for iterating in the multistatement

getMoreResults() Returns true if the next result is a ResultSet object; false if it is an update count or there are no more results

Sorry I'm slightly confused by the expectation of the two statements above. If there is a multi statement query where none of the queries will return a result set, this should return false for each statement even if there are more statements available in the multistatement?

SFStatement returns true for getMoreResults() only if the next result has a result set: https://github.com/snowflakedb/snowflake-jdbc/blob/master/src/main/java/net/snowflake/client/core/SFStatement.java#L888

Is the previous commit to check if there are more statements needed if it's not dependent on whether there are more statements but whether the next statement is a resultset?

[JpnTrzdTzURv3lOIQad0]

The asserts committed for this merge request were correct. To phrase it differently: getMoreResults() moves to the next statement and returns true if that statement's output is a ResultSet object. Otherwise it always returns false.

[sfc-gh-inataraj]

Concerns Raised Here:

This is what the jdbc spec says for: java.sql.Statement.getMoreResults

"Moves to this Statement object's next result, returns true if it is a ResultSet object, and implicitly closes any current ResultSetobject(s) obtained with the method getResultSet.

There are no more results when the following is true:

// stmt is a Statement object

 ((stmt.getMoreResults() == false) && (stmt.getUpdateCount() == -1))

Returns:true if the next result is a ResultSetobject; false if it is an update count or there areno more results"

The important piece here is that it requires both the stmt.getMoreResults() AND the stmt.getUpdateCount() to be checked to indicate whether we are at the end of the results, or even whether there are any more results available. The result of getMoreResults method was never intended to make assumptions about the subsequent statements that may or may not exist. It only knows about the statement result that you just retrieved with that call.

The reason why this is necessary, is that most jdbc implementations (eg. SQL Server, Oracle, Postgres) do NOT have a way to "look" ahead to know what the next statement returned as a result until it is retrieved with the getMoreResults call. And in fact, in most cases they don't even know where the end of the list actually is until you hit it. This is because these implementations do not retrieve all their results in a single call (like SnowFlake apparently does), instead, the client requests each result separately with each call to the getMoreResults method. This is the reason why you cannot possibly know that there is a result set in a subsequent statement, because there is no guarantee that it has been retrieved yet until the client calls the getMoreResults method. So, to properly follow the spec as intended, you should never be returning true from getMoreResults unless the result that is being retrieved by executing this call actually contains a result set. This is intentional, since you do not know whether any subsequent statement has a result set because you cannot make the assumption that it has been already retrieved from the server, you must make another call to getMoreResults to retrieve the next result in order to know that for that specific result.