kkhatua commented on a change in pull request #1714: DRILL-7048: Implement JDBC
Statement.setMaxRows() with System Option
URL: https://github.com/apache/drill/pull/1714#discussion_r269763048
##########
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/PreparedStatementTest.java
##########
@@ -462,4 +618,25 @@ public void
testParamSettingWhenUnsupportedTypeSaysUnsupported() throws SQLExcep
}
}
+
+ // Sets the SystemMaxRows option
+ private void setSystemMaxRows(int sysValueToSet) throws SQLException {
Review comment:
There are _getter-setter_ method tests that don't rely on what the `SYSTEM`
value is, so we don't need to acquire explicit locks. Only when a query is
being executed, do we need to ensure that changes to the `SYSTEM` value are
synchronized because that is when `QueryContext` will apply a `SESSION` or
`SYSTEM` value. Using the lock to synchronize the change, indirectly
synchronizes these specific tests as well.
We don't have any other control on the parallelism or order of the tests
within the 2 suites (`PreparedStatementTest` and `StatementTest`)... so using a
common lock across both tests is the safest way to run such tests with an
element of randomness (using the `RANDOMIZER` object).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
