-
Notifications
You must be signed in to change notification settings - Fork 614
Fix array tests #2667
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.
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
Fix array tests #2667
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
|
||
| import com.clickhouse.jdbc.types.Array; | ||
| import com.google.common.collect.ImmutableMap; | ||
| import org.slf4j.Logger; | ||
|
|
@@ -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 { | ||
|
|
@@ -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); | ||
| } | ||
|
|
||
|
|
@@ -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())); | ||
| } | ||
|
|
@@ -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); | ||
| } | ||
|
|
||
|
|
@@ -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); | ||
| } | ||
|
|
||
|
|
@@ -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
|
||
| 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
|
||
| } else if (value instanceof List<?>) { | ||
|
Check warning on line 378 in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java
|
||
| List<?> srcList = (List<?>) value; | ||
| arrayDimensions = new int[Math.max(dimensions - stack.size() - 1, 1)]; | ||
| arrayDimensions[0] = srcList.size(); | ||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The stack-based processing approach could potentially cause a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
| 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
|
||
| } 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the issue in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
| 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
|
||
| } | ||
| } | ||
|
|
||
| public Object getValue(int i) { | ||
| return valueGetter.apply(i); | ||
| } | ||
| } | ||
|
|
||
| private static Object[] arrayToObjectArray(Object array) { | ||
| if (array == null) { | ||
| return null; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't there be a case with nested array as well?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is another test. But your are right. |
||
| 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)); | ||
| } | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
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
dimensionsparameter, it could lead to runtime errors. Consider adding validation or handling for cases where the actual nesting is deeper than expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what "potential" issue?