Skip to content

Commit 94a00cc

Browse files
enqueueenqueue
committed
Incorporate review feedback
1 parent 96f9883 commit 94a00cc

File tree

5 files changed

+79
-109
lines changed

5 files changed

+79
-109
lines changed

‎client-v2/src/main/java/com/clickhouse/client/api/Client.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,9 +1573,7 @@ public CompletableFuture<QueryResponse> query(String sqlQuery, Map<String, Objec
15731573
Supplier<QueryResponse> responseSupplier;
15741574

15751575
if (queryParams != null) {
1576-
settings.setOption(
1577-
HttpAPIClientHelper.KEY_STATEMENT_PARAMS,
1578-
formatQueryParameters(queryParams));
1576+
settings.setOption(HttpAPIClientHelper.KEY_STATEMENT_PARAMS, queryParams);
15791577
}
15801578
final QuerySettings finalSettings = new QuerySettings(buildRequestSettings(settings.getAllSettings()));
15811579
responseSupplier = () -> {
@@ -2102,12 +2100,4 @@ private Map<String, Object> buildRequestSettings(Map<String, Object> opSettings)
21022100
return requestSettings;
21032101
}
21042102

2105-
private Map<String, String> formatQueryParameters(Map<String, Object> queryParams) {
2106-
HashMap<String, String> newMap = new HashMap<>(queryParams.size());
2107-
for (String key : queryParams.keySet()) {
2108-
newMap.put(key, DataTypeUtils.format(queryParams.get(key)));
2109-
}
2110-
return newMap;
2111-
}
2112-
21132103
}

‎client-v2/src/main/java/com/clickhouse/client/api/DataTypeUtils.java‎

Lines changed: 41 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
import java.time.Instant;
44
import java.time.ZoneId;
55
import java.time.format.DateTimeFormatter;
6-
import java.util.Arrays;
6+
import java.time.format.DateTimeFormatterBuilder;
7+
import java.time.temporal.ChronoField;
78
import java.util.Objects;
89

910
import com.clickhouse.data.ClickHouseDataType;
1011

12+
import static java.time.temporal.ChronoField.NANO_OF_SECOND;
13+
1114
public class DataTypeUtils {
1215

1316
/**
@@ -25,90 +28,70 @@ public class DataTypeUtils {
2528
*/
2629
public static DateTimeFormatter DATETIME_WITH_NANOS_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.nnnnnnnnn");
2730

31+
private static final DateTimeFormatter INSTANT_FORMATTER = new DateTimeFormatterBuilder()
32+
.appendValue(ChronoField.INSTANT_SECONDS)
33+
.appendFraction(NANO_OF_SECOND, 9, 9, true)
34+
.toFormatter();
35+
2836
/**
29-
* Formats a Java object for use in SQL statements or as query parameter.
37+
* Formats an {@link Instant} object for use in SQL statements or as query
38+
* parameter.
3039
*
31-
* Note, that this method returns the empty {@link java.lang.String}
32-
* &quot;&quot; for {@code null} objects.
33-
*
34-
* @param object
40+
* @param instant
3541
* the Java object to format
36-
* @return a suitable String representation of {@code object}, or the empty
37-
* String for {@code null} objects
42+
* @return a suitable String representation of {@code instant}
43+
* @throws NullPointerException
44+
* if {@code instant} is null
3845
*/
39-
public static String format(Object object) {
40-
return format(object, null);
46+
public static String formatInstant(Instant instant) {
47+
return formatInstant(instant, null);
4148
}
4249

4350
/**
44-
* Formats a Java object for use in SQL statements or as query parameter.
45-
*
46-
* This method uses the {@code dataTypeHint} parameter to find be best
47-
* suitable format for the object.
51+
* Formats an {@link Instant} object for use in SQL statements or as query
52+
* parameter.
4853
*
49-
* Note, that this method returns the empty {@link java.lang.String}
50-
* &quot;&quot; for {@code null} objects. This might not be the correct
51-
* default value for the {@code dataTypeHint}
54+
* This method uses the {@code dataTypeHint} parameter to find the best
55+
* suitable format for the instant.
5256
*
53-
* @param object
57+
* @param instant
5458
* the Java object to format
5559
* @param dataTypeHint
56-
* the ClickHouse data type {@code object} should be used for
57-
* @return a suitable String representation of {@code object}, or the empty
58-
* String for {@code null} objects
60+
* the ClickHouse data type {@code instant} should be used for
61+
* @return a suitable String representation of {@code instant}
62+
* @throws NullPointerException
63+
* if {@code instant} is null
5964
*/
60-
public static String format(Object object, ClickHouseDataType dataTypeHint) {
61-
return format(object, dataTypeHint, null);
65+
public static String formatInstant(Instant instant, ClickHouseDataType dataTypeHint) {
66+
return formatInstant(instant, dataTypeHint, null);
6267
}
6368

6469
/**
65-
* Formats a Java object for use in SQL statements or as query parameter.
70+
* Formats an {@link Instant} object for use in SQL statements or as query
71+
* parameter.
6672
*
67-
* This method uses the {@code dataTypeHint} parameter to find be best
68-
* suitable format for the object.
73+
* This method uses the {@code dataTypeHint} parameter to find the best
74+
* suitable format for the instant.
6975
*
7076
* For <em>some</em> formatting operations, providing a {@code timeZone} is
71-
* mandatory: When formatting time-zone-based values (e.g.
72-
* {@link java.time.OffsetDateTime}, {@link java.time.ZonedDateTime}, etc.)
73-
* for use as ClickHouse data types which are not time-zone-based, e.g.
74-
* {@link ClickHouseDataType#Date}, or vice-versa when formatting
75-
* non-time-zone-based Java objects (e.g. {@link java.time.LocalDateTime} or
76-
* any {@link java.util.Calendar} based objects without time-zone) for use
77-
* as time-zone-based ClickHouse data types, e.g.
78-
* {@link ClickHouseDataType#DateTime64}. Although the ClickHouse server
79-
* might understand simple wall-time Strings (&quot;2025-08-20 13:37:42&quot;)
80-
* even for those data types, it is preferable to use timestamp values.
81-
*
82-
* Note, that this method returns the empty {@link java.lang.String}
83-
* &quot;&quot; for {@code null} objects. This might not be the correct
84-
* default value for {@code dataTypeHint}.
77+
* mandatory, e.g. for {@link ClickHouseDataType#Date}.
8578
*
86-
* @param object
79+
* @param instant
8780
* the Java object to format
8881
* @param dataTypeHint
8982
* the ClickHouse data type {@code object} should be used for
9083
* @param timeZone
91-
* the time zone to be used when formatting time-zone-based Java
92-
* objects for use in non-time-zone-based ClickHouse data types
93-
* and vice versa
84+
* the time zone to be used when formatting the instant for use
85+
* in non-time-zone-based ClickHouse data types
9486
* @return a suitable String representation of {@code object}, or the empty
9587
* String for {@code null} objects
88+
* @throws NullPointerException
89+
* if {@code instant} is null
9690
*/
97-
public static String format(Object object, ClickHouseDataType dataTypeHint,
98-
ZoneId timeZone)
99-
{
100-
if (object == null) {
101-
return "";
102-
}
103-
if (object instanceof Instant) {
104-
return formatInstant((Instant) object, dataTypeHint, timeZone);
105-
}
106-
return String.valueOf(object);
107-
}
108-
109-
private static String formatInstant(Instant instant, ClickHouseDataType dataTypeHint,
91+
public static String formatInstant(Instant instant, ClickHouseDataType dataTypeHint,
11092
ZoneId timeZone)
11193
{
94+
Objects.requireNonNull(instant, "Instant required for formatInstant");
11295
if (dataTypeHint == null) {
11396
return formatInstantDefault(instant);
11497
}
@@ -129,12 +112,7 @@ private static String formatInstant(Instant instant, ClickHouseDataType dataType
129112
}
130113

131114
private static String formatInstantDefault(Instant instant) {
132-
String nanos = String.valueOf(instant.getNano());
133-
char[] n = new char[9];
134-
Arrays.fill(n, '0');
135-
int nanosLength = Math.min(9, nanos.length());
136-
nanos.getChars(0, nanosLength, n, 9 - nanosLength);
137-
return String.valueOf(instant.getEpochSecond()) + "." + new String(n);
115+
return INSTANT_FORMATTER.format(instant);
138116
}
139117

140118
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ private void addQueryParams(URIBuilder req, Map<String, Object> requestConfig) {
572572
}
573573
if (requestConfig.containsKey(KEY_STATEMENT_PARAMS)) {
574574
Map<?, ?> params = (Map<?, ?>) requestConfig.get(KEY_STATEMENT_PARAMS);
575-
params.forEach((k, v) -> req.addParameter("param_" + k, (String.valueOf(v))));
575+
params.forEach((k, v) -> req.addParameter("param_" + k, String.valueOf(v)));
576576
}
577577

578578
boolean clientCompression = ClientConfigProperties.COMPRESS_CLIENT_REQUEST.getOrDefault(requestConfig);

client-v2/src/test/java/com/clickhouse/client/e2e/ParameterizedQueryTest.java renamed to client-v2/src/test/java/com/clickhouse/client/ParameterizedQueryTest.java

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
/*
2-
* Copyright (c) 2025 Riege Software. All rights reserved.
3-
* Use is subject to license terms.
4-
*/
5-
package com.clickhouse.client.e2e;
1+
package com.clickhouse.client;
62

73
import java.time.Instant;
84
import java.time.LocalDateTime;
@@ -26,10 +22,6 @@
2622
import org.testng.annotations.DataProvider;
2723
import org.testng.annotations.Test;
2824

29-
import com.clickhouse.client.BaseIntegrationTest;
30-
import com.clickhouse.client.ClickHouseNode;
31-
import com.clickhouse.client.ClickHouseProtocol;
32-
import com.clickhouse.client.ClickHouseServerForTest;
3325
import com.clickhouse.client.api.Client;
3426
import com.clickhouse.client.api.DataTypeUtils;
3527
import com.clickhouse.client.api.command.CommandSettings;
@@ -45,7 +37,6 @@ public class ParameterizedQueryTest extends BaseIntegrationTest {
4537

4638
@BeforeMethod(groups = {"integration"})
4739
public void setUp() {
48-
ClickHouseNode node = getServer(ClickHouseProtocol.HTTP);
4940
client = newClient().build();
5041
}
5142

@@ -81,14 +72,14 @@ void testParamsWithInstant() throws Exception {
8172
() -> Integer.valueOf(rowId.incrementAndGet() % 2),
8273
() -> testValues.get(rowId.get() % 2).toLocalDate()
8374
.toString(),
84-
() -> DataTypeUtils.format(
75+
() -> DataTypeUtils.formatInstant(
8576
testValues.get(rowId.get() % 2).toInstant(),
8677
ClickHouseDataType.DateTime),
87-
() -> DataTypeUtils.format(
78+
() -> DataTypeUtils.formatInstant(
8879
testValues.get(rowId.get() % 2).toInstant()),
89-
() -> DataTypeUtils.format(
80+
() -> DataTypeUtils.formatInstant(
9081
testValues.get(rowId.get() % 2).toInstant()),
91-
() -> DataTypeUtils.format(
82+
() -> DataTypeUtils.formatInstant(
9283
testValues.get(rowId.get() % 2).toInstant())),
9384
2);
9485

@@ -101,12 +92,14 @@ void testParamsWithInstant() throws Exception {
10192
"INSERT INTO " + tableName + " (id, d, dt, dt64_3, dt64_9, dt64_3_lax) "
10293
+ "VALUES ("
10394
+ rowId.incrementAndGet() + ", "
104-
+ "'" + DataTypeUtils.format(manualTestValue, ClickHouseDataType.Date, tzUTC) + "', "
105-
+ "'" + DataTypeUtils.format(manualTestValue, ClickHouseDataType.DateTime) + "', "
95+
+ "'" + DataTypeUtils.formatInstant(manualTestValue, ClickHouseDataType.Date, tzUTC) + "', "
96+
+ "'" + DataTypeUtils.formatInstant(manualTestValue, ClickHouseDataType.DateTime) + "', "
10697
+ "{manualTestValue:DateTime64}, "
10798
+ "{manualTestValue:DateTime64(9)}, "
10899
+ "{manualTestValue:DateTime64})",
109-
Collections.singletonMap("manualTestValue", manualTestValue));
100+
Collections.singletonMap(
101+
"manualTestValue",
102+
DataTypeUtils.formatInstant(manualTestValue)));
110103

111104
try (QueryResponse response = client.query(
112105
"SELECT * FROM " + tableName + " ORDER by id ASC").get();
@@ -190,7 +183,7 @@ void testParamsWithInstant() throws Exception {
190183
{ "dt", "<", testValues.get(1).toInstant(), -1, new int[] { 0, 1, 2 } },
191184
{ "dt", ">", testValues.get(1).toInstant(), -1, new int[0] },
192185

193-
// dt63_3 column
186+
// dt64_3 column
194187
{ "dt64_3", "=", Instant.now(), -1, new int[0] },
195188
{ "dt64_3", "=", testValues.get(1).toInstant(), -1, new int[]{ 1 } },
196189
{ "dt64_3", "=", testValues.get(1).toInstant().truncatedTo(ChronoUnit.MILLIS),
@@ -199,7 +192,7 @@ void testParamsWithInstant() throws Exception {
199192
-1, new int[] { 0, 2 } },
200193
{ "dt64_3", ">", testValues.get(1).toInstant(), -1, new int[0] },
201194

202-
// dt63_9 column
195+
// dt64_9 column
203196
{ "dt64_9", "=", Instant.now(), 9, new int[0] },
204197
{ "dt64_9", "=", testValues.get(1).toInstant(), 9,
205198
new int[]{ 1 } },
@@ -208,7 +201,7 @@ void testParamsWithInstant() throws Exception {
208201
{ "dt64_9", "<", testValues.get(1).toInstant(), 9, new int[] { 0, 2 } },
209202
{ "dt64_9", ">", testValues.get(1).toInstant(), 9, new int[0] },
210203

211-
// dt63_3_lax column
204+
// dt64_3_lax column
212205
{ "dt64_3_lax", "=", Instant.now(), -1, new int[0] },
213206
{ "dt64_3_lax", "=", testValues.get(1).toInstant(), -1,
214207
new int[]{ 1 } },
@@ -233,7 +226,7 @@ void testParamsWithInstant() throws Exception {
233226
}
234227
}
235228

236-
@Test(dataProvider = "stringParameters")
229+
@Test(groups = {"integration"}, dataProvider = "stringParameters")
237230
void testStringParams(String paramValue) throws Exception {
238231
String table = "test_params_unicode";
239232
String column = "val";
@@ -265,7 +258,7 @@ private int[] queryInstant(String tableName, String fieldName, String operator,
265258
"SELECT id FROM " + tableName + " WHERE " + fieldName + " "
266259
+ operator + " {x:DateTime64" + (scale > 0 ? "(" + scale + ")" : "") + "} "
267260
+ "ORDER by id ASC",
268-
Collections.singletonMap("x", param)).get();
261+
Collections.singletonMap("x", DataTypeUtils.formatInstant(param))).get();
269262
ClickHouseBinaryFormatReader reader = client.newBinaryFormatReader(response))
270263
{
271264
List<Integer> ints = new ArrayList<>(3);
@@ -354,6 +347,7 @@ private static Object[][] createStringParameterValues() {
354347
{ "foo/bar" },
355348
{ "foobar 20" },
356349
{ " leading_and_trailing_spaces " },
350+
// https://github.com/ClickHouse/ClickHouse/issues/70240
357351
// { "multi\nline\r\ndos" },
358352
// { "nicely\"quoted\'string\'" },
359353
};

‎client-v2/src/test/java/com/clickhouse/client/api/DataTypeUtilsTests.java‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,17 @@ void testDateTimeWithNanosFormatter() {
3838

3939
@Test
4040
void formatInstantForDateNullInstant() {
41-
assertEquals("", DataTypeUtils.format(
42-
null, ClickHouseDataType.Date, ZoneId.systemDefault()));
41+
assertThrows(
42+
NullPointerException.class,
43+
() -> DataTypeUtils.formatInstant(null, ClickHouseDataType.Date,
44+
ZoneId.systemDefault()));
4345
}
4446

4547
@Test
4648
void formatInstantForDateNullTimeZone() {
4749
assertThrows(
4850
NullPointerException.class,
49-
() -> DataTypeUtils.format(Instant.now(), ClickHouseDataType.Date, null));
51+
() -> DataTypeUtils.formatInstant(Instant.now(), ClickHouseDataType.Date, null));
5052
}
5153

5254
@Test
@@ -55,21 +57,27 @@ void formatInstantForDate() {
5557
ZoneId tzLAX = ZoneId.of("America/Los_Angeles");
5658
Instant instant = ZonedDateTime.of(
5759
2025, 7, 20, 5, 5, 42, 0, tzBER).toInstant();
58-
assertEquals(DataTypeUtils.format(instant, ClickHouseDataType.Date, tzBER), "2025-07-20");
59-
assertEquals(DataTypeUtils.format(instant, ClickHouseDataType.Date, tzLAX), "2025-07-19");
60+
assertEquals(
61+
DataTypeUtils.formatInstant(instant, ClickHouseDataType.Date, tzBER),
62+
"2025-07-20");
63+
assertEquals(
64+
DataTypeUtils.formatInstant(instant, ClickHouseDataType.Date, tzLAX),
65+
"2025-07-19");
6066
}
6167

6268
@Test
63-
void formatNullValue() {
64-
assertEquals("", DataTypeUtils.format(null));
69+
void formatInstantNullValue() {
70+
assertThrows(
71+
NullPointerException.class,
72+
() -> DataTypeUtils.formatInstant(null));
6573
}
6674

6775
@Test
6876
void formatInstantForDateTime() {
6977
TimeZone tzBER = TimeZone.getTimeZone("Europe/Berlin");
7078
Instant instant = ZonedDateTime.of(
7179
2025, 7, 20, 5, 5, 42, 232323, tzBER.toZoneId()).toInstant();
72-
String formatted = DataTypeUtils.format(instant, ClickHouseDataType.DateTime);
80+
String formatted = DataTypeUtils.formatInstant(instant, ClickHouseDataType.DateTime);
7381
assertEquals(formatted, "1752980742");
7482
assertEquals(
7583
Instant.ofEpochSecond(Long.parseLong(formatted)),
@@ -81,7 +89,7 @@ void formatInstantForDateTime64() {
8189
TimeZone tzBER = TimeZone.getTimeZone("Europe/Berlin");
8290
Instant instant = ZonedDateTime.of(
8391
2025, 7, 20, 5, 5, 42, 232323232, tzBER.toZoneId()).toInstant();
84-
String formatted = DataTypeUtils.format(instant);
92+
String formatted = DataTypeUtils.formatInstant(instant);
8593
assertEquals(formatted, "1752980742.232323232");
8694
String[] formattedParts = formatted.split("\\.");
8795
assertEquals(
@@ -96,7 +104,7 @@ void formatInstantForDateTime64SmallerNanos() {
96104
TimeZone tzBER = TimeZone.getTimeZone("Europe/Berlin");
97105
Instant instant = ZonedDateTime.of(
98106
2025, 7, 20, 5, 5, 42, 23, tzBER.toZoneId()).toInstant();
99-
String formatted = DataTypeUtils.format(instant);
107+
String formatted = DataTypeUtils.formatInstant(instant);
100108
assertEquals(formatted, "1752980742.000000023");
101109
String[] formattedParts = formatted.split("\\.");
102110
assertEquals(
@@ -113,11 +121,11 @@ void formatInstantForDateTime64Truncated() {
113121
Instant instant = ZonedDateTime.of(
114122
2025, 7, 20, 5, 5, 42, 232323232, tzBER.toZoneId()).toInstant();
115123
assertEquals(
116-
DataTypeUtils.format(
124+
DataTypeUtils.formatInstant(
117125
instant.truncatedTo(ChronoUnit.SECONDS)),
118126
"1752980742.000000000");
119127
assertEquals(
120-
DataTypeUtils.format(
128+
DataTypeUtils.formatInstant(
121129
instant.truncatedTo(ChronoUnit.MILLIS)),
122130
"1752980742.232000000");
123131
}

0 commit comments

Comments
 (0)