Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
109 changes: 95 additions & 14 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.clickhouse.data.ClickHouseDataType;
import com.clickhouse.data.Tuple;
import com.clickhouse.data.format.BinaryStreamUtils;
import com.clickhouse.jdbc.PreparedStatementImpl;

Check warning on line 9 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this unused import 'com.clickhouse.jdbc.PreparedStatementImpl'.

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP5&open=AZq9B36Mj3V95VOO4oP5&pullRequest=2667
import com.clickhouse.jdbc.types.Array;
import com.google.common.collect.ImmutableMap;
import org.slf4j.Logger;
Expand All @@ -31,7 +32,9 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Stack;
import java.util.TreeMap;
import java.util.function.Function;
import java.util.stream.Collectors;

public class JdbcUtils {
Expand Down Expand Up @@ -85,6 +88,8 @@
map.put(ClickHouseDataType.LineString, JDBCType.ARRAY);
map.put(ClickHouseDataType.MultiPolygon, JDBCType.ARRAY);
map.put(ClickHouseDataType.MultiLineString, JDBCType.ARRAY);
map.put(ClickHouseDataType.Tuple, JDBCType.OTHER);
map.put(ClickHouseDataType.Nothing, JDBCType.OTHER);
return ImmutableMap.copyOf(map);
}

Expand Down Expand Up @@ -169,6 +174,8 @@
default:
map.put(e.getKey(), Object.class);
}
} else if (e.getValue().equals(JDBCType.STRUCT)) {
map.put(e.getKey(), Object.class);
} else {
map.put(e.getKey(), SQL_TYPE_TO_CLASS_MAP.get(e.getValue()));
}
Expand Down Expand Up @@ -255,12 +262,13 @@
if (value instanceof List<?>) {
List<?> listValue = (List<?>) value;
if (type != java.sql.Array.class) {
return convertList(listValue, type);
return convertList(listValue, type, column.getArrayNestedLevel());
}

if (column != null && column.getArrayBaseColumn() != null) {
ClickHouseDataType baseType = column.getArrayBaseColumn().getDataType();
Object[] convertedValues = convertList(listValue, convertToJavaClass(baseType));
Object[] convertedValues = convertList(listValue, convertToJavaClass(baseType),
column.getArrayNestedLevel());
return new Array(column, convertedValues);
}

Expand All @@ -281,7 +289,8 @@

if (column != null && column.getArrayBaseColumn() != null) {
ClickHouseDataType baseType = column.getArrayBaseColumn().getDataType();
Object[] convertedValues = convertArray(arrayValue.getArrayOfObjects(), convertToJavaClass(baseType));
Object[] convertedValues = convertArray(arrayValue.getArray(), convertToJavaClass(baseType),
column.getArrayNestedLevel());
return new Array(column, convertedValues);
}

Expand Down Expand Up @@ -348,31 +357,103 @@
throw new SQLException("Unsupported conversion from " + value.getClass().getName() + " to " + type.getName(), ExceptionUtils.SQL_STATE_DATA_EXCEPTION);
}

public static <T> T[] convertList(List<?> values, Class<T> type) throws SQLException {
public static <T> T[] convertList(List<?> values, Class<T> type, int dimensions) throws SQLException {
if (values == null) {
return null;
}
if (values.isEmpty()) {
return (T[]) java.lang.reflect.Array.newInstance(type, 0);
}
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, values.size());
for (int i = 0; i < values.size(); i++) {
convertedValues[i] = (T) convert(values.get(i), type);

int[] arrayDimensions = new int[dimensions];
arrayDimensions[0] = values.size();
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
Stack<ArrayProcessingCursor> stack = new Stack<>();

Check warning on line 368 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the synchronized class "Stack" by an unsynchronized one such as "Deque".

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oPy&open=AZq9B36Mj3V95VOO4oPy&pullRequest=2667
stack.push(new ArrayProcessingCursor(convertedValues, values, values.size()));

while (!stack.isEmpty()) {
ArrayProcessingCursor cursor = stack.pop();

for (int i = 0; i < cursor.size; i++) {
Object value = cursor.getValue(i);
if (value == null) {
continue; // no need to set null value

Check warning on line 377 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this redundant jump.

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP0&open=AZq9B36Mj3V95VOO4oP0&pullRequest=2667
} else if (value instanceof List<?>) {

Check warning on line 378 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this instanceof check and cast with 'instanceof List srcList'

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP1&open=AZq9B36Mj3V95VOO4oP1&pullRequest=2667
List<?> srcList = (List<?>) value;
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
arrayDimensions[0] = srcList.size();
Comment on lines +380 to +381
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a potential issue with array dimensions handling. If the actual nesting level of the data exceeds the specified dimensions parameter, it could lead to runtime errors. Consider adding validation or handling for cases where the actual nesting is deeper than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what "potential" issue?

T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
stack.push(new ArrayProcessingCursor(targetArray, value, srcList.size()));
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
} else {
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
}
}
}
Comment on lines +371 to 389
Copy link
Contributor

Choose a reason for hiding this comment

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

The stack-based processing approach could potentially cause a StackOverflowError for deeply nested arrays. Consider adding a maximum depth check or using an iterative approach that's less susceptible to stack overflow issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a iterative approach. Stack used to organize state.


return convertedValues;
}

public static <T> T[] convertArray(Object[] values, Class<T> type) throws SQLException {
/**
* Convert array to java array and all its elements
* @param values
* @param type
* @param dimensions
* @return
* @param <T>
* @throws SQLException
*/
public static <T> T[] convertArray(Object values, Class<T> type, int dimensions) throws SQLException {
if (values == null) {
return null;
}
T[] convertedValues = (T[]) java.lang.reflect.Array.newInstance(type, values.length);
for (int i = 0; i < values.length; i++) {
convertedValues[i] = (T) convert(values[i], type);

int[] arrayDimensions = new 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<>();

Check warning on line 411 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the synchronized class "Stack" by an unsynchronized one such as "Deque".

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oPz&open=AZq9B36Mj3V95VOO4oPz&pullRequest=2667
stack.push(new ArrayProcessingCursor(convertedValues, values, arrayDimensions[0]));

while (!stack.isEmpty()) {
ArrayProcessingCursor cursor = stack.pop();

for (int i = 0; i < cursor.size; i++) {
Object value = cursor.getValue(i);
if (value == null) {
continue; // no need to set null value

Check warning on line 420 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this redundant jump.

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP2&open=AZq9B36Mj3V95VOO4oP2&pullRequest=2667
} else if (value.getClass().isArray()) {
arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)];
arrayDimensions[0] = java.lang.reflect.Array.getLength(value);
Comment on lines +422 to +423
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the issue in convertList, the array dimensions calculation in convertArray might not handle all edge cases correctly. Consider adding more robust dimension detection or error handling for mismatched array structures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what cases?

T[] targetArray = (T[]) java.lang.reflect.Array.newInstance(type, arrayDimensions);
stack.push(new ArrayProcessingCursor(targetArray, value, arrayDimensions[0]));
java.lang.reflect.Array.set(cursor.targetArray, i, targetArray);
} else {
java.lang.reflect.Array.set(cursor.targetArray, i, convert(value, type));
}
}
}

return convertedValues;
}

private static final class ArrayProcessingCursor {
private final Object targetArray;
private final int size;
private final Function<Integer, Object> valueGetter;

public ArrayProcessingCursor(Object targetArray, Object srcArray, int size) {
this.targetArray = targetArray;
this.size = size;
if (srcArray instanceof List<?>) {

Check warning on line 444 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this instanceof check and cast with 'instanceof List list'

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP3&open=AZq9B36Mj3V95VOO4oP3&pullRequest=2667
List<?> list = (List<?>) srcArray;
this.valueGetter = list::get;
} else {
this.valueGetter = (i) -> java.lang.reflect.Array.get(srcArray, i);

Check warning on line 448 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the parentheses around the "i" parameter

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZq9B36Mj3V95VOO4oP4&open=AZq9B36Mj3V95VOO4oP4&pullRequest=2667
}
}

public Object getValue(int i) {
return valueGetter.apply(i);
}
}

private static Object[] arrayToObjectArray(Object array) {
if (array == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,30 +50,33 @@ public void testConvertToArray() throws Exception {
@Test(groups = {"unit"})
public void testConvertArray() throws Exception {
// primitive classes are unwrapped
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Boolean.class), new Boolean[] { false, true });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Byte.class), new Byte[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Short.class), new Short[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Integer.class), new Integer[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Long.class), new Long[] { 0L, 1L });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Float.class), new Float[] { 0.0f, 1.0f });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Double.class), new Double[] { 0.0, 1.0 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, String.class), new String[] { "0", "1" });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, BigDecimal.class), new BigDecimal[] { BigDecimal.valueOf(0), BigDecimal.valueOf(1) });
assertNull(JdbcUtils.convertArray(null, Integer.class));
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Boolean.class, 1), new Boolean[] { false, true });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Byte.class, 1), new Byte[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Short.class, 1), new Short[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Integer.class, 1), new Integer[] { 0, 1 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Long.class, 1), new Long[] { 0L, 1L });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Float.class, 1), new Float[] { 0.0f, 1.0f });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, Double.class, 1), new Double[] { 0.0, 1.0 });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, String.class, 1), new String[] { "0", "1" });
assertEquals(JdbcUtils.convertArray(new Object[] { 0, 1 }, BigDecimal.class, 1), new BigDecimal[] { BigDecimal.valueOf(0), BigDecimal.valueOf(1) });
assertEquals(JdbcUtils.convertArray(new Object[][] { new Object[] {1, 2, 3}, new Object[] { 4, 5, 6} }, String.class, 2),
new String[][] { new String[] {"1", "2", "3"}, new String[] {"4", "5", "6"} });

assertNull(JdbcUtils.convertArray(null, Integer.class, 1));
}


@Test(groups = {"unit"})
public void testConvertList() throws Exception {
ClickHouseColumn column = ClickHouseColumn.of("arr", "Array(Int32)");
List<Integer> src = Arrays.asList(1, 2, 3);
Integer[] dst = JdbcUtils.convertList(src, Integer.class);
Integer[] dst = JdbcUtils.convertList(src, Integer.class, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't there be a case with nested array as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is another test. But your are right.
We plan to revisit this code and will do more testing then.

assertEquals(dst.length, src.size());
assertEquals(dst[0], src.get(0));
assertEquals(dst[1], src.get(1));
assertEquals(dst[2], src.get(2));

assertNull(JdbcUtils.convertList(null, Integer.class));
assertNull(JdbcUtils.convertList(null, Integer.class, 1));
}


Expand Down
Loading