[jdbc-v2] Fixed overriding server settings in JDBC Connection

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.9.9
+
+
+### Bug Fixes
+- **[jdbc-v2]** Fixed issues with hardcoded `async_insert` settings in `ConnectionImpl`. Now it let override them. (https://github.com/ClickHouse/clickhouse-java/issues/2652, https://github.com/ClickHouse/clickhouse-java/issues/2825)
+
 ## 0.9.8
 
 ### Improvements 

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

@@ -46,4 +46,9 @@ public final class ServerSettings {
     public static final String ASYNC_INSERT = "async_insert";
 
     public static final String WAIT_ASYNC_INSERT = "wait_for_async_insert";
+
+    // Misc
+    public static final String ON = "1";
+
+    public static final String OFF = "0";
 }

jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java

@@ -2,7 +2,7 @@
 
 import com.clickhouse.client.api.Client;
 import com.clickhouse.client.api.ClientConfigProperties;
 import com.clickhouse.client.api.internal.ServerSettings;
 import com.clickhouse.client.api.metadata.TableSchema;
 import com.clickhouse.client.api.query.GenericRecord;
 import com.clickhouse.client.api.query.QuerySettings;
@@ -110,9 +110,7 @@
                 this.client.loadServerInfo();
             }
             this.schema = client.getDefaultDatabase();
-            this.defaultQuerySettings = new QuerySettings()
-                    .serverSetting(ServerSettings.ASYNC_INSERT, "0")
-                    .serverSetting(ServerSettings.WAIT_END_OF_QUERY, "0");
+            this.defaultQuerySettings = new QuerySettings();
 
             this.metadata = new DatabaseMetaDataImpl(this, false, url);
             this.defaultCalendar = Calendar.getInstance();

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java

@@ -3,6 +3,7 @@
 import com.clickhouse.client.api.Client;
 import com.clickhouse.client.api.ClientConfigProperties;
 import com.clickhouse.client.api.http.ClickHouseHttpProto;
+import com.clickhouse.client.api.internal.ServerSettings;
 import com.clickhouse.data.ClickHouseDataType;
 import com.clickhouse.jdbc.Driver;
 import com.clickhouse.jdbc.DriverProperties;
@@ -89,7 +90,7 @@
 
         Map<String, String> urlProperties = parseUrl(url);
         String tmpConnectionUrl = urlProperties.remove(PARSE_URL_CONN_URL_PROP);
-        initProperties(urlProperties, props);
+        buildFinalProperties(urlProperties, props);
 
         // after initializing all properties - set final connection URL
         boolean useSSLInfo = Boolean.parseBoolean(props.getProperty(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
@@ -266,10 +267,31 @@
         return properties;
     }
 
-    private void initProperties(Map<String, String> urlProperties, Properties providedProperties) {
+    /**
+     * Creates initial properties with defaults.
+     * @return mutable HashMap with default values.
+     */
+    Map<String, String> createProperties() {
+        Map<String, String> props = new HashMap<>();
+
+        // Requires to wait result on insert
+        props.put(DriverProperties.serverSetting(ServerSettings.ASYNC_INSERT), ServerSettings.OFF);
+
+        // Requires to wait result of query (manly insert)
+        props.put(DriverProperties.serverSetting(ServerSettings.WAIT_END_OF_QUERY), ServerSettings.ON);
+
+        return props;
+    }
+
+    /**
+     * Combines url properties and provided ones via {@link java.sql.Driver#connect(String, Properties)}
+     * @param urlProperties - properties parsed from URL
+     * @param providedProperties - properties object provided by application
+     */
+    private void buildFinalProperties(Map<String, String> urlProperties, Properties providedProperties) {
 
         // Copy provided properties
-        Map<String, String> props = new HashMap<>();
+        Map<String, String> props = createProperties();
         // Set driver properties defaults (client will do the same)
         for (DriverProperties prop : DriverProperties.values()) {
             if (prop.getDefaultValue() != null) {

jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 
 import static org.testng.Assert.assertEquals;
@@ -264,6 +265,64 @@ public void testExecuteUpdateDates() throws Exception {
         }
     }
 
+    private static final int ASYNC_INSERT_SETTINGS_DP_ROWS = 100_000;
+
+    @DataProvider(name = "asyncInsertSettingsDP")
+    public static Object[][] asyncInsertSettingsDP() {
+        return new Object[][]{
+                // asyncInsert, waitEndOfQuery, expectedUpdateCount, expectedSelectCount
+                {ServerSettings.OFF, ServerSettings.OFF, ASYNC_INSERT_SETTINGS_DP_ROWS, ASYNC_INSERT_SETTINGS_DP_ROWS},
+                {ServerSettings.OFF, ServerSettings.ON, ASYNC_INSERT_SETTINGS_DP_ROWS, ASYNC_INSERT_SETTINGS_DP_ROWS},
+                {ServerSettings.ON, ServerSettings.OFF, 0, -1},
+                {ServerSettings.ON, ServerSettings.ON, ASYNC_INSERT_SETTINGS_DP_ROWS, ASYNC_INSERT_SETTINGS_DP_ROWS}
+        };
+    }
+
+    @Test(groups = {"integration"}, dataProvider = "asyncInsertSettingsDP")
+    public void testInsertWithAsyncInsert(String asyncInsert, String waitAsyncInsert, int expectedUpdateCount, int expectedSelectCount) throws Exception {
+        String tableName = "test_async_insert_param_" + asyncInsert + "_" + waitAsyncInsert + "_" + UUID.randomUUID().toString().replace("-", "_");
+
+        Properties props = new Properties();
+        props.setProperty(ClientConfigProperties.serverSetting(ServerSettings.ASYNC_INSERT), asyncInsert);
+        props.setProperty(ClientConfigProperties.serverSetting(ServerSettings.WAIT_ASYNC_INSERT), waitAsyncInsert);
+        // Wait end of query off for isolation of this logic
+        props.setProperty(ClientConfigProperties.serverSetting(ServerSettings.WAIT_END_OF_QUERY), ServerSettings.OFF);
+
+        if (waitAsyncInsert.equals(ServerSettings.ON)) {
+            // make it flash to disk to check that we get result. If buffer is bigger server may wait flashing to disk.
+            props.setProperty(ClientConfigProperties.serverSetting("async_insert_max_data_size"), "3488890");
+        }
+
+        try (Connection conn = getJdbcConnection(props)) {
+            try (Statement stmt = conn.createStatement()) {
+                stmt.execute("CREATE TABLE IF NOT EXISTS " + getDatabase() + "." + tableName + " (id UInt32, name String, value Float64, status Int8, timestamp DateTime) ENGINE = MergeTree ORDER BY id");
+                stmt.execute("TRUNCATE TABLE " + getDatabase() + "." + tableName);
+
+                StringBuilder sb = new StringBuilder("INSERT INTO " + getDatabase() + "." + tableName + " FORMAT TSV\n");
+                for (int i = 0; i < ASYNC_INSERT_SETTINGS_DP_ROWS; i++) {
+                    sb.append(i).append("\t")
+                      .append("name_").append(i).append("\t")
+                      .append(i * 1.1).append("\t")
+                      .append(i % 2).append("\t")
+                      .append("2023-01-01 10:11:12").append("\n");
+                }
+
+                int updateCount = stmt.executeUpdate(sb.toString());
+                assertEquals(updateCount, expectedUpdateCount);
+
+                try (ResultSet rs = stmt.executeQuery("SELECT count() FROM " + getDatabase() + "." + tableName)) {
+                    assertTrue(rs.next());
+                    int count = rs.getInt(1);
+                    if (expectedSelectCount == -1) {
+                        assertTrue(count < ASYNC_INSERT_SETTINGS_DP_ROWS, "Expected count to be < " + ASYNC_INSERT_SETTINGS_DP_ROWS + ", but was: " + count);
+                    } else {
+                        assertEquals(count, expectedSelectCount);
+                    }
+                }
+            }
+        }
+    }
+
 
     @Test(groups = {"integration"})
     public void testExecuteUpdateBatch() throws Exception {

jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/JdbcConfigurationTest.java

@@ -121,8 +121,11 @@ public void testParseURLValid(String jdbcURL, Properties properties,
             throws Exception
     {
         JdbcConfiguration configuration = new JdbcConfiguration(jdbcURL, properties);
+        // We need to copy defaults to expected.
+        Map<String, String> expectedWithDefaults = configuration.createProperties();
+        expectedWithDefaults.putAll(expectedClientProps);
         assertEquals(configuration.getConnectionUrl(), connectionURL, "URL: " + jdbcURL);
-        assertEquals(configuration.clientProperties, expectedClientProps, "URL: " + jdbcURL);
+        assertEquals(configuration.clientProperties, expectedWithDefaults, "URL: " + jdbcURL);
         Client.Builder bob = new Client.Builder();
         configuration.applyClientProperties(bob);
         Client client = bob.build();