Skip to content

fixed issue converting multidimensional lists and arrays #2735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chernser wants to merge 1 commit into
base: main
Choose a base branch
from
+155 −13
Open

fixed issue converting multidimensional lists and arrays #2735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,148 @@ public void testDates() throws Exception {
}
}

@Test(groups = {"integration"}, dataProvider = "testNestedArrays_dp")
public void testNestedArrays(String columnDef, String insertValues, String[] expectedStrValues,
String[] expectedListValues) throws Exception {
final String table = "test_nested_arrays";
client.execute("DROP TABLE IF EXISTS " + table).get();
client.execute(tableDefinition(table, "rowId Int32", columnDef)).get();

String insertSQL = "INSERT INTO " + table + " VALUES " + insertValues;
try (QueryResponse response = client.query(insertSQL).get()) {}
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using client.query(...) for an INSERT is unconventional and may add unnecessary overhead versus an execute-style API (if available in this module). If the client provides execute() / command() for non-SELECT statements, prefer that here to better match intent and reduce reliance on query semantics for DML.

Suggested change
try (QueryResponse response = client.query(insertSQL).get()) {}
client.execute(insertSQL).get();

Copilot uses AI. Check for mistakes.

List<GenericRecord> records = client.queryAll("SELECT * FROM " + table + " ORDER BY rowId");
Assert.assertEquals(records.size(), expectedStrValues.length);

for (GenericRecord record : records) {
int rowId = record.getInteger("rowId");

// Check getString() - includes quotes for string values
String actualValue = record.getString("arr");
Assert.assertEquals(actualValue, expectedStrValues[rowId - 1],
"getString() mismatch at row " + rowId + " for " + columnDef);

// Check getObject() - should return an ArrayValue
Object objValue = record.getObject("arr");
Assert.assertNotNull(objValue, "getObject() returned null at row " + rowId);
Assert.assertTrue(objValue instanceof BinaryStreamReader.ArrayValue,
"getObject() should return ArrayValue at row " + rowId + ", got: " + objValue.getClass().getName());
BinaryStreamReader.ArrayValue arrayValue = (BinaryStreamReader.ArrayValue) objValue;
Assert.assertEquals(arrayValue.asList().toString(), expectedListValues[rowId - 1],
"getObject().asList() mismatch at row " + rowId + " for " + columnDef);

// Check getList() - should return a List representation (no quotes for strings)
List<?> listValue = record.getList("arr");
Assert.assertNotNull(listValue, "getList() returned null at row " + rowId);
Assert.assertEquals(listValue.toString(), expectedListValues[rowId - 1],
"getList() mismatch at row " + rowId + " for " + columnDef);
}
}

@DataProvider
public Object[][] testNestedArrays_dp() {
return new Object[][] {
// 2D arrays of integers - Array(Array(Int32))
{
"arr Array(Array(Int32))",
"(1, [[1, 2], [3, 4]]), (2, [[5, 6, 7]]), (3, [[]]), (4, [[8], [], [9, 10]]), " +
"(5, [[11]]), (6, [[12, 13], [14, 15]]), (7, [[100, 200]]), (8, [[16]]), (9, [[17, 18]]), (10, [[19, 20, 21]])",
new String[] {
"[[1, 2], [3, 4]]", "[[5, 6, 7]]", "[[]]", "[[8], [], [9, 10]]",
"[[11]]", "[[12, 13], [14, 15]]", "[[100, 200]]", "[[16]]", "[[17, 18]]", "[[19, 20, 21]]"
},
new String[] {
"[[1, 2], [3, 4]]", "[[5, 6, 7]]", "[[]]", "[[8], [], [9, 10]]",
"[[11]]", "[[12, 13], [14, 15]]", "[[100, 200]]", "[[16]]", "[[17, 18]]", "[[19, 20, 21]]"
}
},
// 2D arrays of strings - Array(Array(String))
{
"arr Array(Array(String))",
"(1, [['a', 'b'], ['c', 'd']]), (2, [['hello', 'world']]), (3, [[]]), (4, [['x'], [], ['y', 'z']]), " +
"(5, [['test']]), (6, [['foo', 'bar']]), (7, [['single']]), (8, [['alpha', 'beta']]), (9, [['one']]), (10, [['end']])",
new String[] { // getString() format - with quotes
"[['a', 'b'], ['c', 'd']]", "[['hello', 'world']]", "[[]]", "[['x'], [], ['y', 'z']]",
"[['test']]", "[['foo', 'bar']]", "[['single']]", "[['alpha', 'beta']]", "[['one']]", "[['end']]"
},
new String[] { // getList() format - no quotes
"[[a, b], [c, d]]", "[[hello, world]]", "[[]]", "[[x], [], [y, z]]",
"[[test]]", "[[foo, bar]]", "[[single]]", "[[alpha, beta]]", "[[one]]", "[[end]]"
}
},
// 3D arrays of integers - Array(Array(Array(Int32)))
{
"arr Array(Array(Array(Int32)))",
"(1, [[[1, 2], [3]]]), (2, [[[4], [5, 6]], [[7]]]), (3, [[[]]]), (4, [[[8, 9]]]), " +
"(5, [[[10], [11, 12]]]), (6, [[[13]]]), (7, [[[14, 15], [16]]]), (8, [[[17]]]), (9, [[[18, 19]]]), (10, [[[]]])",
new String[] {
"[[[1, 2], [3]]]", "[[[4], [5, 6]], [[7]]]", "[[[]]]", "[[[8, 9]]]",
"[[[10], [11, 12]]]", "[[[13]]]", "[[[14, 15], [16]]]", "[[[17]]]", "[[[18, 19]]]", "[[[]]]"
},
new String[] {
"[[[1, 2], [3]]]", "[[[4], [5, 6]], [[7]]]", "[[[]]]", "[[[8, 9]]]",
"[[[10], [11, 12]]]", "[[[13]]]", "[[[14, 15], [16]]]", "[[[17]]]", "[[[18, 19]]]", "[[[]]]"
}
},
// 2D arrays of floats - Array(Array(Float64))
{
"arr Array(Array(Float64))",
"(1, [[1.1, 2.2], [3.3]]), (2, [[4.4]]), (3, [[5.5, 6.6, 7.7]]), (4, [[]]), " +
"(5, [[8.8]]), (6, [[9.9, 10.1]]), (7, [[11.2]]), (8, [[12.3, 13.4]]), (9, [[14.5]]), (10, [[15.6, 16.7]])",
new String[] {
"[[1.1, 2.2], [3.3]]", "[[4.4]]", "[[5.5, 6.6, 7.7]]", "[[]]",
"[[8.8]]", "[[9.9, 10.1]]", "[[11.2]]", "[[12.3, 13.4]]", "[[14.5]]", "[[15.6, 16.7]]"
},
new String[] {
"[[1.1, 2.2], [3.3]]", "[[4.4]]", "[[5.5, 6.6, 7.7]]", "[[]]",
"[[8.8]]", "[[9.9, 10.1]]", "[[11.2]]", "[[12.3, 13.4]]", "[[14.5]]", "[[15.6, 16.7]]"
}
},
// 3D arrays of strings - Array(Array(Array(String)))
{
"arr Array(Array(Array(String)))",
"(1, [[['a', 'b']]]), (2, [[['c'], ['d', 'e']]]), (3, [[[]]]), (4, [[['f']]]), " +
"(5, [[['g', 'h']]]), (6, [[['i']]]), (7, [[['a', 'b'], ['c']], [['d', 'e', 'f']]]), (8, [[[]]]), (9, [[['m']]]), (10, [[['n', 'o']]])",
new String[] { // getString() format - with quotes
"[[['a', 'b']]]", "[[['c'], ['d', 'e']]]", "[[[]]]", "[[['f']]]",
"[[['g', 'h']]]", "[[['i']]]", "[[['a', 'b'], ['c']], [['d', 'e', 'f']]]", "[[[]]]", "[[['m']]]", "[[['n', 'o']]]"
},
new String[] { // getList() format - no quotes
"[[[a, b]]]", "[[[c], [d, e]]]", "[[[]]]", "[[[f]]]",
"[[[g, h]]]", "[[[i]]]", "[[[a, b], [c]], [[d, e, f]]]", "[[[]]]", "[[[m]]]", "[[[n, o]]]"
}
},
// 2D arrays of nullable integers - Array(Array(Nullable(Int32)))
{
"arr Array(Array(Nullable(Int32)))",
"(1, [[1, NULL, 2]]), (2, [[NULL]]), (3, [[3, 4, NULL]]), (4, [[]]), " +
"(5, [[NULL, NULL]]), (6, [[5]]), (7, [[6, NULL]]), (8, [[NULL, 7]]), (9, [[8, 9]]), (10, [[NULL]])",
new String[] {
"[[1, NULL, 2]]", "[[NULL]]", "[[3, 4, NULL]]", "[[]]",
"[[NULL, NULL]]", "[[5]]", "[[6, NULL]]", "[[NULL, 7]]", "[[8, 9]]", "[[NULL]]"
},
new String[] {
"[[1, null, 2]]", "[[null]]", "[[3, 4, null]]", "[[]]",
"[[null, null]]", "[[5]]", "[[6, null]]", "[[null, 7]]", "[[8, 9]]", "[[null]]"
}
},
// 4D arrays of integers - Array(Array(Array(Array(Int32))))
{
"arr Array(Array(Array(Array(Int32))))",
"(1, [[[[1, 2]]]]), (2, [[[[3], [4, 5]]]]), (3, [[[[]]]]), (4, [[[[6]]]]), " +
"(5, [[[[7, 8]]]]), (6, [[[[9]]]]), (7, [[[[10, 11]]]]), (8, [[[[]]]]), (9, [[[[12]]]]), (10, [[[[13, 14]]]])",
new String[] {
"[[[[1, 2]]]]", "[[[[3], [4, 5]]]]", "[[[[]]]]", "[[[[6]]]]",
"[[[[7, 8]]]]", "[[[[9]]]]", "[[[[10, 11]]]]", "[[[[]]]]", "[[[[12]]]]", "[[[[13, 14]]]]"
},
new String[] {
"[[[[1, 2]]]]", "[[[[3], [4, 5]]]]", "[[[[]]]]", "[[[[6]]]]",
"[[[[7, 8]]]]", "[[[[9]]]]", "[[[[10, 11]]]]", "[[[[]]]]", "[[[[12]]]]", "[[[[13, 14]]]]"
}
}
};
}

public static String tableDefinition(String table, String... columns) {
StringBuilder sb = new StringBuilder();
sb.append("CREATE TABLE " + table + " ( ");
Expand Down
19 changes: 11 additions & 8 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,7 @@ public static <T> T[] convertList(List<?> values, Class<T> type, int dimensions)
arrayDimensions[0] = values.size();
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
Stack<ArrayProcessingCursor> stack = new Stack<>();
stack.push(new ArrayProcessingCursor(convertedValues, values, values.size()));

stack.push(new ArrayProcessingCursor(convertedValues, values, values.size(), dimensions));
while (!stack.isEmpty()) {
ArrayProcessingCursor cursor = stack.pop();

Expand All @@ -422,10 +421,11 @@ public static <T> T[] convertList(List<?> values, Class<T> type, int dimensions)
continue; // no need to set null value
} else if (value instanceof List<?>) {
List<?> srcList = (List<?>) value;
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
int depth = cursor.depth - 1;
arrayDimensions = new int[depth];
arrayDimensions[0] = srcList.size();
Comment on lines +424 to 426
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can throw at runtime when depth becomes 0 (or negative): new int[0] is legal, but then arrayDimensions[0] = ... will always throw ArrayIndexOutOfBoundsException. The previous implementation clamped the array length to at least 1. Consider restoring a clamp/guard (e.g., ensure the dimension array has length >= 1) and/or throwing a clear exception when the actual nesting exceeds the declared dimensions.

Copilot uses AI. Check for mistakes.
T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
stack.push(new ArrayProcessingCursor(targetArray, value, srcList.size()));
stack.push(new ArrayProcessingCursor(targetArray, value, srcList.size(), depth));
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
} else {
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
Expand Down Expand Up @@ -454,7 +454,7 @@ public static <T> T[] convertArray(Object values, Class<T> type, int dimensions)
arrayDimensions[0] = java.lang.reflect.Array.getLength(values);
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
Stack<ArrayProcessingCursor> stack = new Stack<>();
stack.push(new ArrayProcessingCursor(convertedValues, values, arrayDimensions[0]));
stack.push(new ArrayProcessingCursor(convertedValues, values, arrayDimensions[0], dimensions));

while (!stack.isEmpty()) {
ArrayProcessingCursor cursor = stack.pop();
Expand All @@ -464,10 +464,11 @@ public static <T> T[] convertArray(Object values, Class<T> type, int dimensions)
if (value == null) {
continue; // no need to set null value
} else if (value.getClass().isArray()) {
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
int depth = cursor.depth - 1;
arrayDimensions = new int[depth];
arrayDimensions[0] = java.lang.reflect.Array.getLength(value);
Comment on lines +467 to 469
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem as convertList: if cursor.depth is 1 when encountering a nested array, depth becomes 0 and indexing arrayDimensions[0] will throw. Please reintroduce a minimum length guard (or a deterministic error) so malformed or unexpectedly-shaped inputs don’t fail with an opaque ArrayIndexOutOfBoundsException.

Copilot uses AI. Check for mistakes.
T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
stack.push(new ArrayProcessingCursor(targetArray, value, arrayDimensions[0]));
stack.push(new ArrayProcessingCursor(targetArray, value, arrayDimensions[0], depth));
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
} else {
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
Expand All @@ -482,10 +483,12 @@ private static final class ArrayProcessingCursor {
private final Object targetArray;
private final int size;
private final Function<Integer, Object> valueGetter;
private final int depth;

public ArrayProcessingCursor(Object targetArray, Object srcArray, int size) {
public ArrayProcessingCursor(Object targetArray, Object srcArray, int size, int depth) {
this.targetArray = targetArray;
this.size = size;
this.depth = depth;
if (srcArray instanceof List<?>) {
List<?> list = (List<?>) srcArray;
this.valueGetter = list::get;
Expand Down
7 changes: 2 additions & 5 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,11 +1219,6 @@ public void testNestedArrays() throws Exception {
}
}

/**
* Test for https://github.com/ClickHouse/clickhouse-java/issues/2723
* getString() on nested arrays was failing with NullPointerException due to re-entrancy bug
* in DataTypeConverter when converting nested arrays to string representation.
*/
@Test(groups = { "integration" })
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Javadoc explaining the linked regression (issue #2723 and the prior NPE/re-entrancy context) was removed. Since this test reads like a regression test, keeping a short comment with the issue link and the failure mode would help future maintainers understand why these exact assertions exist.

Suggested change
@Test(groups = { "integration" })
@Test(groups = { "integration" })
/**
* Regression test for nested array stringification and {@code getString()} on
* array-typed expressions (see issue #2723).
* <p>
* Originally, calling {@code ResultSet.getString()} on:
* <ul>
* <li>nested array columns such as {@code Array(Array(Int32))}, and</li>
* <li>array-returning expressions used inside {@code CASE/WHEN}</li>
* </ul>
* could throw a {@link NullPointerException} due to re-entrancy issues in
* the driver. This test verifies that {@code getString()} now works
* correctly for those scenarios.
*/

Copilot uses AI. Check for mistakes.
public void testNestedArrayToString() throws SQLException {
// Test 1: Simple nested array - getString on Array(Array(Int32))
Expand Down Expand Up @@ -1273,6 +1268,8 @@ public void testNestedArrayToString() throws SQLException {
assertTrue(rs.next());
String result = rs.getString("deep_nested");
assertEquals(result, "[[['a', 'b'], ['c']], [['d', 'e', 'f']]]");
Array arr = rs.getArray(1);
assertEquals(arr.getArray(), new String[][][] {{{"a", "b"}, {"c"}}, {{ "d", "e", "f"}}});
Comment on lines +1271 to +1272
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues here: (1) rs.getArray(1) is index-based and may not refer to deep_nested if the SELECT column order changes; prefer rs.getArray(\"deep_nested\") for stability. (2) assertEquals(arr.getArray(), new String[][][] {...}) is likely to fail because multi-dimensional arrays usually require deep comparison; use a deep equality check (e.g., Arrays.deepEquals) or compare a deep string representation. Also consider freeing the java.sql.Array (e.g., arr.free()) to avoid resource leaks in integration tests.

Suggested change
Array arr = rs.getArray(1);
assertEquals(arr.getArray(), new String[][][] {{{"a", "b"}, {"c"}}, {{ "d", "e", "f"}}});
Array arr = rs.getArray("deep_nested");
String[][][] actual = (String[][][]) arr.getArray();
String[][][] expected = new String[][][] {{{"a", "b"}, {"c"}}, {{"d", "e", "f"}}};
assertTrue(Arrays.deepEquals(actual, expected));
arr.free();

Copilot uses AI. Check for mistakes.
}
}
}
Expand Down
Loading