fix: executeBatch did not respect statement timeout#871
Conversation
The Statement#executeBatch() method did not respect the statement
timeout that had been set on a statement. This meant that a batch like
the following would always use the default timeout that is set in the
Gapic generated client, and ignore the timeout set in
Statement#setTimeout(int):
statement.setTimeout(120);
statement.addBatch("insert into foo (id) values (1)");
statement.executeBatch(); // This ignored the statement timeout
Fixes #870
product-auto-label
Bot
added
size: m
rajatbhatta
left a comment
rajatbhatta
left a comment
There was a problem hiding this comment.
LGTM, just had one query.
| <exclude>com.google.cloud.spanner.jdbc.it.**</exclude> | ||
| <exclude>com.google.cloud.spanner.jdbc.JdbcStatementTimeoutTest</exclude> |
There was a problem hiding this comment.
Why did we exclude the tests under it/ and the new test that we added this in PR (JdbcStatementTimeoutTest)?
There was a problem hiding this comment.
The integration tests should already have been excluded, but the existing exclude didn't actually work (it references a Category that is not used). The reason that the integration tests weren't executed by the maven-surefire-plugin was because of the default shared configuration that excluded them. So in theory we could remove the exclude here. But as it was already present and intentional, but not working, I decided to fix it so it does work.
The reason that we are excluding the StatementTimeoutTest here is that it is relatively slow. I felt that it was overkill to execute the test every time you execute mvn test, which is already taking quite long for this library, and instead only run it together with the integration tests. So the test is executed by the CI, but not locally unless you explicitly enable it.
3 participants
The
Statement#executeBatch()method did not respect the statement timeout that had been set on a statement. This meant that a batch like the following would always use the default timeout that is set in the Gapic generated client, and ignore the timeout set inStatement#setTimeout(int):Fixes #870