Skip to content

Commit f496078

Browse files
chernserchernser
committed
Added more tests to verify places where unknown datatype may break things and improved messages
1 parent b242955 commit f496078

File tree

3 files changed

+209
-8
lines changed

3 files changed

+209
-8
lines changed

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
package com.clickhouse.client.api.internal;
22

3+
import com.clickhouse.client.api.ClientException;
34
import com.clickhouse.client.api.metadata.TableSchema;
45
import com.clickhouse.data.ClickHouseColumn;
6+
import org.slf4j.Logger;
7+
import org.slf4j.LoggerFactory;
58

6-
import java.io.BufferedReader;
7-
import java.io.IOException;
8-
import java.io.InputStream;
9-
import java.io.InputStreamReader;
10-
import java.io.StringReader;
9+
import java.io.*;
1110
import java.util.ArrayList;
1211
import java.util.List;
1312
import java.util.Properties;
@@ -23,7 +22,14 @@ public static TableSchema readTSKV(InputStream content, String table, String sql
2322
p.clear();
2423
if (!line.trim().isEmpty()) {
2524
p.load(new StringReader(line.replaceAll("\t", "\n")));
26-
ClickHouseColumn column = ClickHouseColumn.of(p.getProperty("name"), p.getProperty("type"));
25+
final String columnName = p.getProperty("name");
26+
final String columnType = p.getProperty("type");
27+
ClickHouseColumn column;
28+
try {
29+
column = ClickHouseColumn.of(columnName, p.getProperty("type"));
30+
} catch (IllegalArgumentException e) {
31+
throw new ClientException("Failed parse column `"+ columnName + "` definition of type '" + columnType + "'", e);
32+
}
2733
String defaultType = p.getProperty("default_type");
2834
String defaultExpression = p.getProperty("default_expression");
2935
column.setHasDefault(defaultType != null && !defaultType.isEmpty());

‎client-v2/src/test/java/com/clickhouse/client/metadata/MetadataTests.java‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414
import com.clickhouse.client.api.internal.ServerSettings;
1515
import com.clickhouse.client.api.metadata.DefaultColumnToMethodMatchingStrategy;
1616
import com.clickhouse.client.api.metadata.TableSchema;
17+
import com.clickhouse.client.api.command.CommandSettings;
18+
import com.clickhouse.client.api.query.GenericRecord;
1719
import com.clickhouse.client.api.query.QuerySettings;
1820
import com.clickhouse.data.ClickHouseColumn;
1921
import com.clickhouse.data.ClickHouseDataType;
2022
import com.clickhouse.data.ClickHouseRecord;
23+
import com.clickhouse.data.ClickHouseVersion;
24+
import org.apache.commons.lang3.StringUtils;
2125
import org.testng.Assert;
2226
import org.testng.annotations.BeforeMethod;
2327
import org.testng.annotations.DataProvider;
2428
import org.testng.annotations.Test;
2529

30+
import java.util.HashSet;
2631
import java.util.Iterator;
2732
import java.util.List;
33+
import java.util.Set;
2834

2935
public class MetadataTests extends BaseIntegrationTest {
3036

@@ -106,6 +112,192 @@ public Object[][] testMatchingNormalizationData() {
106112
};
107113
}
108114

115+
@Test(groups = {"integration"})
116+
public void testCreateTableWithAllDataTypes() throws Exception {
117+
String tableName = "test_all_data_types";
118+
119+
// Query system.data_type_families to get all known types
120+
List<GenericRecord> dbTypes = client.queryAll("SELECT name, alias_to FROM system.data_type_families ORDER BY name");
121+
122+
// Types that cannot be used directly in CREATE TABLE columns
123+
Set<String> excludedTypes = new HashSet<>();
124+
excludedTypes.add("AggregateFunction");
125+
excludedTypes.add("SimpleAggregateFunction");
126+
excludedTypes.add("Nothing");
127+
excludedTypes.add("Nullable"); // Nullable is a wrapper, not a base type
128+
excludedTypes.add("LowCardinality"); // LowCardinality is a wrapper, not a base type
129+
excludedTypes.add("Enum"); // Enum is a base type, use Enum8 or Enum16 instead
130+
131+
// Build column definitions
132+
StringBuilder createTableSql = new StringBuilder();
133+
createTableSql.append("CREATE TABLE IF NOT EXISTS ").append(tableName).append(" (");
134+
135+
int columnIndex = 0;
136+
Set<String> addedTypes = new HashSet<>();
137+
138+
for (GenericRecord dbType : dbTypes) {
139+
String typeName = dbType.getString("name");
140+
String aliasTo = dbType.getString("alias_to");
141+
142+
// Use alias if available, otherwise use the name
143+
String actualType = StringUtils.isNotBlank(aliasTo) ? aliasTo : typeName;
144+
145+
// Skip excluded types and duplicates
146+
if (excludedTypes.contains(actualType) || addedTypes.contains(actualType)) {
147+
continue;
148+
}
149+
150+
// Generate column name and type definition
151+
String columnName = "col_" + columnIndex++;
152+
String columnType = getColumnTypeDefinition(actualType);
153+
154+
if (columnType != null) {
155+
createTableSql.append(columnName).append(" ").append(columnType).append(", ");
156+
addedTypes.add(actualType);
157+
}
158+
}
159+
160+
// Remove trailing comma and space
161+
if (createTableSql.length() > 0 && createTableSql.charAt(createTableSql.length() - 2) == ',') {
162+
createTableSql.setLength(createTableSql.length() - 2);
163+
}
164+
165+
createTableSql.append(") ENGINE = Memory");
166+
167+
// Create table with appropriate settings for experimental types
168+
CommandSettings commandSettings = new CommandSettings();
169+
// Allow Geometry type which may have variant ambiguity
170+
commandSettings.serverSetting("allow_suspicious_variant_types", "1");
171+
// Allow QBit experimental type
172+
commandSettings.serverSetting("allow_experimental_qbit_type", "1");
173+
try {
174+
// Try to enable experimental types if version supports them
175+
if (isVersionMatch("[24.8,)")) {
176+
commandSettings.serverSetting("allow_experimental_variant_type", "1")
177+
.serverSetting("allow_experimental_dynamic_type", "1")
178+
.serverSetting("allow_experimental_json_type", "1");
179+
}
180+
if (isVersionMatch("[25.8,)")) {
181+
commandSettings.serverSetting("enable_time_time64_type", "1");
182+
}
183+
} catch (Exception e) {
184+
// If version check fails, continue without experimental settings
185+
}
186+
187+
try {
188+
client.execute("DROP TABLE IF EXISTS " + tableName).get().close();
189+
client.execute(createTableSql.toString(), commandSettings).get().close();
190+
191+
// Verify the schema
192+
TableSchema schema = client.getTableSchema(tableName);
193+
Assert.assertNotNull(schema, "Schema should not be null");
194+
Assert.assertEquals(schema.getTableName(), tableName);
195+
Assert.assertTrue(schema.getColumns().size() > 0, "Table should have at least one column");
196+
197+
// Verify that we have columns for the types we added
198+
// Some types might fail to create, so we check for at least 80% success rate
199+
Assert.assertTrue(schema.getColumns().size() >= addedTypes.size() * 0.8,
200+
"Expected at least 80% of types to be successfully created. Created: " + schema.getColumns().size() + ", Attempted: " + addedTypes.size());
201+
} finally {
202+
try {
203+
client.execute("DROP TABLE IF EXISTS " + tableName).get().close();
204+
} catch (Exception e) {
205+
// Ignore cleanup errors
206+
}
207+
}
208+
}
209+
210+
/**
211+
* Returns the column type definition for a given ClickHouse type name.
212+
* Returns null if the type cannot be used in CREATE TABLE.
213+
*/
214+
private String getColumnTypeDefinition(String typeName) {
215+
// Handle types that need parameters
216+
switch (typeName) {
217+
case "Decimal":
218+
return "Decimal(10, 2)";
219+
case "Decimal32":
220+
return "Decimal32(2)";
221+
case "Decimal64":
222+
return "Decimal64(3)";
223+
case "Decimal128":
224+
return "Decimal128(4)";
225+
case "Decimal256":
226+
return "Decimal256(5)";
227+
case "DateTime":
228+
return "DateTime";
229+
case "DateTime32":
230+
return "DateTime32";
231+
case "DateTime64":
232+
return "DateTime64(3)";
233+
case "FixedString":
234+
return "FixedString(10)";
235+
case "Enum":
236+
// Enum base type cannot be used without parameters, return null to skip it
237+
return null;
238+
case "Enum8":
239+
return "Enum8('a' = 1, 'b' = 2)";
240+
case "Enum16":
241+
return "Enum16('a' = 1, 'b' = 2)";
242+
case "Array":
243+
return "Array(String)";
244+
case "Map":
245+
return "Map(String, Int32)";
246+
case "Tuple":
247+
return "Tuple(String, Int32)";
248+
case "Nested":
249+
return "Nested(name String, value Int32)";
250+
case "Object":
251+
return "Object('json' String)";
252+
case "Variant":
253+
return "Variant(String, Int32)";
254+
case "Dynamic":
255+
return "Dynamic";
256+
case "JSON":
257+
return "JSON";
258+
case "IntervalYear":
259+
return "IntervalYear";
260+
case "IntervalQuarter":
261+
return "IntervalQuarter";
262+
case "IntervalMonth":
263+
return "IntervalMonth";
264+
case "IntervalWeek":
265+
return "IntervalWeek";
266+
case "IntervalDay":
267+
return "IntervalDay";
268+
case "IntervalHour":
269+
return "IntervalHour";
270+
case "IntervalMinute":
271+
return "IntervalMinute";
272+
case "IntervalSecond":
273+
return "IntervalSecond";
274+
case "IntervalMicrosecond":
275+
return "IntervalMicrosecond";
276+
case "IntervalMillisecond":
277+
return "IntervalMillisecond";
278+
case "IntervalNanosecond":
279+
return "IntervalNanosecond";
280+
case "Time":
281+
return "Time";
282+
case "Time64":
283+
return "Time64";
284+
case "QBit":
285+
// QBit requires two parameters: element type and number of elements
286+
return "QBit(Float32, 4)";
287+
case "Geometry":
288+
case "Geometry1":
289+
// Geometry type (requires allow_suspicious_variant_types = 1 setting)
290+
return "Geometry";
291+
default:
292+
// For simple types without parameters, return as-is
293+
return typeName;
294+
}
295+
}
296+
297+
public boolean isVersionMatch(String versionExpression) {
298+
List<GenericRecord> serverVersion = client.queryAll("SELECT version()");
299+
return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression);
300+
}
109301
protected Client.Builder newClient() {
110302
ClickHouseNode node = getServer(ClickHouseProtocol.HTTP);
111303
boolean isSecure = isCloud();

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,9 +1125,12 @@ public ResultSet getTypeInfo() throws SQLException {
11251125
if (type == null) {
11261126
try {
11271127
type = JdbcUtils.convertToSqlType(ClickHouseDataType.valueOf(typeName));
1128-
} catch (Exception e) {
1129-
log.error("Failed to convert column data type to SQL type: {}", typeName, e);
1128+
} catch (IllegalArgumentException e) {
1129+
log.error("Unknown type: " + typeName + ". Please check for a new version of the client.");
11301130
type = JDBCType.OTHER; // In case of error, return SQL type 0
1131+
} catch (Exception e) {
1132+
log.error("Failed to get SQL type for type: " + typeName, e);
1133+
type = JDBCType.OTHER;
11311134
}
11321135
}
11331136

0 commit comments

Comments
 (0)