Skip to content

Commit 25e39a9

Browse files
chernserchernser
authored
Merge pull request #2735 from ClickHouse/02/04/26/client_fix_arrays
fixed issue converting multidimensional lists and arrays
2 parents c0ab16d + 6e22f09 commit 25e39a9

4 files changed

Lines changed: 177 additions & 22 deletions

File tree

‎client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java‎

Lines changed: 141 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@
66
import com.clickhouse.client.ClickHouseServerForTest;
77
import com.clickhouse.client.api.Client;
88
import com.clickhouse.client.api.ClientException;
9-
import com.clickhouse.client.api.DataTypeUtils;
109
import com.clickhouse.client.api.command.CommandSettings;
1110
import com.clickhouse.client.api.data_formats.ClickHouseBinaryFormatReader;
1211
import com.clickhouse.client.api.data_formats.internal.BinaryStreamReader;
13-
import com.clickhouse.client.api.data_formats.internal.SerializerUtils;
1412
import com.clickhouse.client.api.enums.Protocol;
1513
import com.clickhouse.client.api.insert.InsertSettings;
1614
import com.clickhouse.client.api.metadata.TableSchema;
@@ -34,19 +32,13 @@
3432
import java.lang.reflect.Method;
3533
import java.math.BigDecimal;
3634
import java.math.RoundingMode;
37-
import java.sql.Connection;
3835
import java.time.Instant;
3936
import java.time.LocalDate;
4037
import java.time.LocalDateTime;
41-
import java.time.LocalTime;
4238
import java.time.Period;
4339
import java.time.ZoneId;
4440
import java.time.ZoneOffset;
45-
import java.time.ZonedDateTime;
46-
import java.time.temporal.ChronoField;
4741
import java.time.temporal.ChronoUnit;
48-
import java.time.temporal.TemporalAccessor;
49-
import java.time.temporal.TemporalField;
5042
import java.util.ArrayList;
5143
import java.util.Arrays;
5244
import java.util.Collections;
@@ -1145,6 +1137,147 @@ public void testDates() throws Exception {
11451137
}
11461138
}
11471139

1140+
@Test(groups = {"integration"}, dataProvider = "testNestedArrays_dp")
1141+
public void testNestedArrays(String columnDef, String insertValues, String[] expectedStrValues,
1142+
String[] expectedListValues) throws Exception {
1143+
final String table = "test_nested_arrays";
1144+
client.execute("DROP TABLE IF EXISTS " + table).get();
1145+
client.execute(tableDefinition(table, "rowId Int32", columnDef)).get();
1146+
1147+
client.execute("INSERT INTO " + table + " VALUES " + insertValues).get().close();
1148+
1149+
List<GenericRecord> records = client.queryAll("SELECT * FROM " + table + " ORDER BY rowId");
1150+
Assert.assertEquals(records.size(), expectedStrValues.length);
1151+
1152+
for (GenericRecord record : records) {
1153+
int rowId = record.getInteger("rowId");
1154+
1155+
// Check getString() - includes quotes for string values
1156+
String actualValue = record.getString("arr");
1157+
Assert.assertEquals(actualValue, expectedStrValues[rowId - 1],
1158+
"getString() mismatch at row " + rowId + " for " + columnDef);
1159+
1160+
// Check getObject() - should return an ArrayValue
1161+
Object objValue = record.getObject("arr");
1162+
Assert.assertNotNull(objValue, "getObject() returned null at row " + rowId);
1163+
Assert.assertTrue(objValue instanceof BinaryStreamReader.ArrayValue,
1164+
"getObject() should return ArrayValue at row " + rowId + ", got: " + objValue.getClass().getName());
1165+
BinaryStreamReader.ArrayValue arrayValue = (BinaryStreamReader.ArrayValue) objValue;
1166+
Assert.assertEquals(arrayValue.asList().toString(), expectedListValues[rowId - 1],
1167+
"getObject().asList() mismatch at row " + rowId + " for " + columnDef);
1168+
1169+
// Check getList() - should return a List representation (no quotes for strings)
1170+
List<?> listValue = record.getList("arr");
1171+
Assert.assertNotNull(listValue, "getList() returned null at row " + rowId);
1172+
Assert.assertEquals(listValue.toString(), expectedListValues[rowId - 1],
1173+
"getList() mismatch at row " + rowId + " for " + columnDef);
1174+
}
1175+
}
1176+
1177+
@DataProvider
1178+
public Object[][] testNestedArrays_dp() {
1179+
return new Object[][] {
1180+
// 2D arrays of integers - Array(Array(Int32))
1181+
{
1182+
"arr Array(Array(Int32))",
1183+
"(1, [[1, 2], [3, 4]]), (2, [[5, 6, 7]]), (3, [[]]), (4, [[8], [], [9, 10]]), " +
1184+
"(5, [[11]]), (6, [[12, 13], [14, 15]]), (7, [[100, 200]]), (8, [[16]]), (9, [[17, 18]]), (10, [[19, 20, 21]])",
1185+
new String[] {
1186+
"[[1, 2], [3, 4]]", "[[5, 6, 7]]", "[[]]", "[[8], [], [9, 10]]",
1187+
"[[11]]", "[[12, 13], [14, 15]]", "[[100, 200]]", "[[16]]", "[[17, 18]]", "[[19, 20, 21]]"
1188+
},
1189+
new String[] {
1190+
"[[1, 2], [3, 4]]", "[[5, 6, 7]]", "[[]]", "[[8], [], [9, 10]]",
1191+
"[[11]]", "[[12, 13], [14, 15]]", "[[100, 200]]", "[[16]]", "[[17, 18]]", "[[19, 20, 21]]"
1192+
}
1193+
},
1194+
// 2D arrays of strings - Array(Array(String))
1195+
{
1196+
"arr Array(Array(String))",
1197+
"(1, [['a', 'b'], ['c', 'd']]), (2, [['hello', 'world']]), (3, [[]]), (4, [['x'], [], ['y', 'z']]), " +
1198+
"(5, [['test']]), (6, [['foo', 'bar']]), (7, [['single']]), (8, [['alpha', 'beta']]), (9, [['one']]), (10, [['end']])",
1199+
new String[] { // getString() format - with quotes
1200+
"[['a', 'b'], ['c', 'd']]", "[['hello', 'world']]", "[[]]", "[['x'], [], ['y', 'z']]",
1201+
"[['test']]", "[['foo', 'bar']]", "[['single']]", "[['alpha', 'beta']]", "[['one']]", "[['end']]"
1202+
},
1203+
new String[] { // getList() format - no quotes
1204+
"[[a, b], [c, d]]", "[[hello, world]]", "[[]]", "[[x], [], [y, z]]",
1205+
"[[test]]", "[[foo, bar]]", "[[single]]", "[[alpha, beta]]", "[[one]]", "[[end]]"
1206+
}
1207+
},
1208+
// 3D arrays of integers - Array(Array(Array(Int32)))
1209+
{
1210+
"arr Array(Array(Array(Int32)))",
1211+
"(1, [[[1, 2], [3]]]), (2, [[[4], [5, 6]], [[7]]]), (3, [[[]]]), (4, [[[8, 9]]]), " +
1212+
"(5, [[[10], [11, 12]]]), (6, [[[13]]]), (7, [[[14, 15], [16]]]), (8, [[[17]]]), (9, [[[18, 19]]]), (10, [[[]]])",
1213+
new String[] {
1214+
"[[[1, 2], [3]]]", "[[[4], [5, 6]], [[7]]]", "[[[]]]", "[[[8, 9]]]",
1215+
"[[[10], [11, 12]]]", "[[[13]]]", "[[[14, 15], [16]]]", "[[[17]]]", "[[[18, 19]]]", "[[[]]]"
1216+
},
1217+
new String[] {
1218+
"[[[1, 2], [3]]]", "[[[4], [5, 6]], [[7]]]", "[[[]]]", "[[[8, 9]]]",
1219+
"[[[10], [11, 12]]]", "[[[13]]]", "[[[14, 15], [16]]]", "[[[17]]]", "[[[18, 19]]]", "[[[]]]"
1220+
}
1221+
},
1222+
// 2D arrays of floats - Array(Array(Float64))
1223+
{
1224+
"arr Array(Array(Float64))",
1225+
"(1, [[1.1, 2.2], [3.3]]), (2, [[4.4]]), (3, [[5.5, 6.6, 7.7]]), (4, [[]]), " +
1226+
"(5, [[8.8]]), (6, [[9.9, 10.1]]), (7, [[11.2]]), (8, [[12.3, 13.4]]), (9, [[14.5]]), (10, [[15.6, 16.7]])",
1227+
new String[] {
1228+
"[[1.1, 2.2], [3.3]]", "[[4.4]]", "[[5.5, 6.6, 7.7]]", "[[]]",
1229+
"[[8.8]]", "[[9.9, 10.1]]", "[[11.2]]", "[[12.3, 13.4]]", "[[14.5]]", "[[15.6, 16.7]]"
1230+
},
1231+
new String[] {
1232+
"[[1.1, 2.2], [3.3]]", "[[4.4]]", "[[5.5, 6.6, 7.7]]", "[[]]",
1233+
"[[8.8]]", "[[9.9, 10.1]]", "[[11.2]]", "[[12.3, 13.4]]", "[[14.5]]", "[[15.6, 16.7]]"
1234+
}
1235+
},
1236+
// 3D arrays of strings - Array(Array(Array(String)))
1237+
{
1238+
"arr Array(Array(Array(String)))",
1239+
"(1, [[['a', 'b']]]), (2, [[['c'], ['d', 'e']]]), (3, [[[]]]), (4, [[['f']]]), " +
1240+
"(5, [[['g', 'h']]]), (6, [[['i']]]), (7, [[['a', 'b'], ['c']], [['d', 'e', 'f']]]), (8, [[[]]]), (9, [[['m']]]), (10, [[['n', 'o']]])",
1241+
new String[] { // getString() format - with quotes
1242+
"[[['a', 'b']]]", "[[['c'], ['d', 'e']]]", "[[[]]]", "[[['f']]]",
1243+
"[[['g', 'h']]]", "[[['i']]]", "[[['a', 'b'], ['c']], [['d', 'e', 'f']]]", "[[[]]]", "[[['m']]]", "[[['n', 'o']]]"
1244+
},
1245+
new String[] { // getList() format - no quotes
1246+
"[[[a, b]]]", "[[[c], [d, e]]]", "[[[]]]", "[[[f]]]",
1247+
"[[[g, h]]]", "[[[i]]]", "[[[a, b], [c]], [[d, e, f]]]", "[[[]]]", "[[[m]]]", "[[[n, o]]]"
1248+
}
1249+
},
1250+
// 2D arrays of nullable integers - Array(Array(Nullable(Int32)))
1251+
{
1252+
"arr Array(Array(Nullable(Int32)))",
1253+
"(1, [[1, NULL, 2]]), (2, [[NULL]]), (3, [[3, 4, NULL]]), (4, [[]]), " +
1254+
"(5, [[NULL, NULL]]), (6, [[5]]), (7, [[6, NULL]]), (8, [[NULL, 7]]), (9, [[8, 9]]), (10, [[NULL]])",
1255+
new String[] {
1256+
"[[1, NULL, 2]]", "[[NULL]]", "[[3, 4, NULL]]", "[[]]",
1257+
"[[NULL, NULL]]", "[[5]]", "[[6, NULL]]", "[[NULL, 7]]", "[[8, 9]]", "[[NULL]]"
1258+
},
1259+
new String[] {
1260+
"[[1, null, 2]]", "[[null]]", "[[3, 4, null]]", "[[]]",
1261+
"[[null, null]]", "[[5]]", "[[6, null]]", "[[null, 7]]", "[[8, 9]]", "[[null]]"
1262+
}
1263+
},
1264+
// 4D arrays of integers - Array(Array(Array(Array(Int32))))
1265+
{
1266+
"arr Array(Array(Array(Array(Int32))))",
1267+
"(1, [[[[1, 2]]]]), (2, [[[[3], [4, 5]]]]), (3, [[[[]]]]), (4, [[[[6]]]]), " +
1268+
"(5, [[[[7, 8]]]]), (6, [[[[9]]]]), (7, [[[[10, 11]]]]), (8, [[[[]]]]), (9, [[[[12]]]]), (10, [[[[13, 14]]]])",
1269+
new String[] {
1270+
"[[[[1, 2]]]]", "[[[[3], [4, 5]]]]", "[[[[]]]]", "[[[[6]]]]",
1271+
"[[[[7, 8]]]]", "[[[[9]]]]", "[[[[10, 11]]]]", "[[[[]]]]", "[[[[12]]]]", "[[[[13, 14]]]]"
1272+
},
1273+
new String[] {
1274+
"[[[[1, 2]]]]", "[[[[3], [4, 5]]]]", "[[[[]]]]", "[[[[6]]]]",
1275+
"[[[[7, 8]]]]", "[[[[9]]]]", "[[[[10, 11]]]]", "[[[[]]]]", "[[[[12]]]]", "[[[[13, 14]]]]"
1276+
}
1277+
}
1278+
};
1279+
}
1280+
11481281
public static String tableDefinition(String table, String... columns) {
11491282
StringBuilder sb = new StringBuilder();
11501283
sb.append("CREATE TABLE " + table + " ( ");

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -408,12 +408,15 @@ public static <T> T[] convertList(List<?> values, Class<T> type, int dimensions)
408408
return null;
409409
}
410410

411+
if (dimensions <= 0) {
412+
throw new IllegalArgumentException("Cannot convert list to array with less then 1D");
413+
}
414+
411415
int[] arrayDimensions = new int[dimensions];
412416
arrayDimensions[0] = values.size();
413417
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
414418
Stack<ArrayProcessingCursor> stack = new Stack<>();
415-
stack.push(new ArrayProcessingCursor(convertedValues, values, values.size()));
416-
419+
stack.push(new ArrayProcessingCursor(convertedValues, values, values.size(), dimensions));
417420
while (!stack.isEmpty()) {
418421
ArrayProcessingCursor cursor = stack.pop();
419422

@@ -423,10 +426,14 @@ public static <T> T[] convertList(List<?> values, Class<T> type, int dimensions)
423426
continue; // no need to set null value
424427
} else if (value instanceof List<?>) {
425428
List<?> srcList = (List<?>) value;
426-
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
429+
int depth = cursor.depth - 1;
430+
if (depth <= 0) {
431+
throw new IllegalStateException("There is a child array at depth 0 where it is not expected");
432+
}
433+
arrayDimensions = new int[depth];
427434
arrayDimensions[0] = srcList.size();
428435
T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
429-
stack.push(new ArrayProcessingCursor(targetArray, value, srcList.size()));
436+
stack.push(new ArrayProcessingCursor(targetArray, value, srcList.size(), depth));
430437
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
431438
} else {
432439
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
@@ -451,11 +458,15 @@ public static <T> T[] convertArray(Object values, Class<T> type, int dimensions)
451458
return null;
452459
}
453460

461+
if (dimensions <= 0) {
462+
throw new IllegalArgumentException("Cannot convert list to array with less then 1D");
463+
}
464+
454465
int[] arrayDimensions = new int[dimensions];
455466
arrayDimensions[0] = java.lang.reflect.Array.getLength(values);
456467
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
457468
Stack<ArrayProcessingCursor> stack = new Stack<>();
458-
stack.push(new ArrayProcessingCursor(convertedValues, values, arrayDimensions[0]));
469+
stack.push(new ArrayProcessingCursor(convertedValues, values, arrayDimensions[0], dimensions));
459470

460471
while (!stack.isEmpty()) {
461472
ArrayProcessingCursor cursor = stack.pop();
@@ -465,10 +476,14 @@ public static <T> T[] convertArray(Object values, Class<T> type, int dimensions)
465476
if (value == null) {
466477
continue; // no need to set null value
467478
} else if (value.getClass().isArray()) {
468-
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
479+
int depth = cursor.depth - 1;
480+
if (depth <= 0) {
481+
throw new IllegalStateException("There is a child array at depth 0 where it is not expected");
482+
}
483+
arrayDimensions = new int[depth];
469484
arrayDimensions[0] = java.lang.reflect.Array.getLength(value);
470485
T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
471-
stack.push(new ArrayProcessingCursor(targetArray, value, arrayDimensions[0]));
486+
stack.push(new ArrayProcessingCursor(targetArray, value, arrayDimensions[0], depth));
472487
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
473488
} else {
474489
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
@@ -483,10 +498,12 @@ private static final class ArrayProcessingCursor {
483498
private final Object targetArray;
484499
private final int size;
485500
private final Function<Integer, Object> valueGetter;
501+
private final int depth;
486502

487-
public ArrayProcessingCursor(Object targetArray, Object srcArray, int size) {
503+
public ArrayProcessingCursor(Object targetArray, Object srcArray, int size, int depth) {
488504
this.targetArray = targetArray;
489505
this.size = size;
506+
this.depth = depth;
490507
if (srcArray instanceof List<?>) {
491508
List<?> list = (List<?>) srcArray;
492509
this.valueGetter = list::get;

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,11 +1250,6 @@ public void testNestedArrays() throws Exception {
12501250
}
12511251
}
12521252

1253-
/**
1254-
* Test for https://github.com/ClickHouse/clickhouse-java/issues/2723
1255-
* getString() on nested arrays was failing with NullPointerException due to re-entrancy bug
1256-
* in DataTypeConverter when converting nested arrays to string representation.
1257-
*/
12581253
@Test(groups = { "integration" })
12591254
public void testNestedArrayToString() throws SQLException {
12601255
// Test 1: Simple nested array - getString on Array(Array(Int32))
@@ -1304,6 +1299,8 @@ public void testNestedArrayToString() throws SQLException {
13041299
assertTrue(rs.next());
13051300
String result = rs.getString("deep_nested");
13061301
assertEquals(result, "[[['a', 'b'], ['c']], [['d', 'e', 'f']]]");
1302+
Array arr = rs.getArray(1);
1303+
assertTrue(Arrays.deepEquals((String[][][])arr.getArray(), new String[][][] {{{"a", "b"}, {"c"}}, {{ "d", "e", "f"}}}));
13071304
}
13081305
}
13091306
}

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/JdbcUtilsTest.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44
import com.clickhouse.data.ClickHouseColumn;
55
import com.clickhouse.data.ClickHouseDataType;
66
import org.testng.Assert;
7-
import org.testng.annotations.*;
7+
import org.testng.annotations.DataProvider;
8+
import org.testng.annotations.Test;
89

910
import java.io.InputStream;
1011
import java.math.BigDecimal;
1112
import java.sql.SQLException;
1213
import java.time.Instant;
1314
import java.time.ZoneId;
1415
import java.util.Arrays;
16+
import java.util.Collections;
1517
import java.util.List;
1618
import java.util.zip.CRC32;
1719

@@ -67,6 +69,9 @@ public void testConvertArray() throws Exception {
6769
new String[][] { new String[] {"1", "2", "3"}, new String[] {"4", "5", "6"} });
6870

6971
assertNull(JdbcUtils.convertArray(null, Integer.class, 1));
72+
73+
Assert.assertThrows(IllegalArgumentException.class, () -> JdbcUtils.convertArray(new Object[0], String.class, 0 ));
74+
Assert.assertThrows(IllegalStateException.class, () -> JdbcUtils.convertArray(new Object[] { new Object[] { 1, 2, 3}}, String.class, 1 ));
7075
}
7176

7277

@@ -81,6 +86,9 @@ public void testConvertList() throws Exception {
8186
assertEquals(dst[2], src.get(2));
8287

8388
assertNull(JdbcUtils.convertList(null, Integer.class, 1));
89+
90+
Assert.assertThrows(IllegalArgumentException.class, () -> JdbcUtils.convertList(Collections.emptyList(), String.class, 0));
91+
Assert.assertThrows(IllegalStateException.class, () -> JdbcUtils.convertList(Arrays.asList(Collections.singletonList(1)), String.class, 1));
8492
}
8593

8694

0 commit comments

Comments
 (0)