Merge pull request #2752 from ClickHouse/02/20/26/fix_max_execution_time

[client-v2] Implemented settings max execution time as server setting

Author: chernser

client-v2/src/main/java/com/clickhouse/client/api/internal/ServerSettings.java

@@ -37,6 +37,12 @@ public final class ServerSettings {
 
     public static final String RESULT_OVERFLOW_MODE_BREAK  = "break";
 
+    /**
+     * Maximum query execution time in seconds on server. 0 means no limit.
+     * If query is not finished in this time then server will send an exception.
+     */
+    public static final String MAX_EXECUTION_TIME = "max_execution_time";
+
     public static final String ASYNC_INSERT = "async_insert";
 
     public static final String WAIT_ASYNC_INSERT = "wait_for_async_insert";

client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java

@@ -117,12 +117,14 @@ public ClickHouseFormat getFormat() {
      * If query is not finished in this time then server will send an exception.
      */
     public QuerySettings setMaxExecutionTime(Integer maxExecutionTime) {
-        settings.setOption("max_execution_time", maxExecutionTime);
+        serverSetting(ServerSettings.MAX_EXECUTION_TIME, String.valueOf(maxExecutionTime));
         return this;
     }
 
     public Integer getMaxExecutionTime() {
-        return (Integer) settings.getOption("max_execution_time");
+        String val = (String) settings.getOption(
+                ClientConfigProperties.serverSetting(ServerSettings.MAX_EXECUTION_TIME));
+        return val == null ? null : Integer.valueOf(val);
     }
 
     /**

client-v2/src/test/java/com/clickhouse/client/SettingsTests.java

@@ -2,6 +2,7 @@
 
 import com.clickhouse.client.api.ClientConfigProperties;
 import com.clickhouse.client.api.insert.InsertSettings;
+import com.clickhouse.client.api.internal.ServerSettings;
 import com.clickhouse.client.api.query.QuerySettings;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -81,8 +82,11 @@ void testQuerySettingsSpecific() throws Exception {
 
         {
             final QuerySettings settings = new QuerySettings();
-            settings.setMaxExecutionTime(10000);
-            Assert.assertEquals(settings.getMaxExecutionTime(), 10000);
+            int val = 10000;
+            settings.setMaxExecutionTime(val);
+            Assert.assertEquals(settings.getMaxExecutionTime(), val);
+            Assert.assertEquals(settings.getAllSettings().get(
+                    ClientConfigProperties.serverSetting(ServerSettings.MAX_EXECUTION_TIME)), String.valueOf(val));
         }
 
         {

client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java

@@ -8,7 +8,6 @@
 import com.clickhouse.client.ClickHouseServerForTest;
 import com.clickhouse.client.api.Client;
 import com.clickhouse.client.api.ClientException;
-import com.clickhouse.client.api.DataTypeUtils;
 import com.clickhouse.client.api.ServerException;
 import com.clickhouse.client.api.command.CommandSettings;
 import com.clickhouse.client.api.data_formats.ClickHouseBinaryFormatReader;
@@ -59,10 +58,10 @@
 import java.net.InetAddress;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
-import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -2151,4 +2150,24 @@ public void testDuplicateColumnNames() throws Exception {
             Assert.assertEquals(record.getString(2), "another name");
         }
     }
+
+    @Test(groups = {"integration"})
+    public void testMaxExecutionTime() throws Exception {
+        try (Client localClient = newClient()
+                .setSocketTimeout(10, ChronoUnit.SECONDS)
+                .build()) {
+
+            QuerySettings settings = new QuerySettings().setMaxExecutionTime(1);
+
+            localClient.query("SELECT sleep(2)", settings).get(10, TimeUnit.SECONDS);
+            Assert.fail("Expected ServerException due to max_execution_time");
+        } catch (ServerException e) {
+            Assert.assertEquals(e.getCode(), 159, "Expected TIMEOUT_EXCEEDED error code");
+        } catch (ExecutionException e) {
+            Assert.assertTrue(e.getCause() instanceof ServerException,
+                    "Expected cause to be ServerException but was: " + e.getCause().getClass().getName());
+            ServerException se = (ServerException) e.getCause();
+            Assert.assertEquals(se.getCode(), 159, "Expected TIMEOUT_EXCEEDED error code");
+        }
+    }
 }