Skip to content

Commit

Permalink
Merge pull request #2105 from ClickHouse/v2_fix_random_issue_0122
Browse files Browse the repository at this point in the history
[client-v2] Small fixes
  • Loading branch information
chernser authored Jan 30, 2025
2 parents fdf6430 + f1a1476 commit 93dfb96
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.clickhouse.data.ClickHouseFormat;
import org.apache.hc.core5.concurrent.DefaultThreadFactory;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpStatus;
import org.slf4j.Logger;
Expand Down Expand Up @@ -1639,8 +1640,13 @@ public CompletableFuture<QueryResponse> query(String sqlQuery, Map<String, Objec
.getFirstHeader(ClickHouseHttpProto.HEADER_QUERY_ID), finalSettings.getQueryId());
metrics.setQueryId(queryId);
metrics.operationComplete();
Header formatHeader = httpResponse.getFirstHeader(ClickHouseHttpProto.HEADER_FORMAT);
ClickHouseFormat responseFormat = finalSettings.getFormat();
if (formatHeader != null) {
responseFormat = ClickHouseFormat.valueOf(formatHeader.getValue());
}

return new QueryResponse(httpResponse, finalSettings.getFormat(), finalSettings, metrics);
return new QueryResponse(httpResponse, responseFormat, finalSettings, metrics);

} catch (Exception e) {
lastException = httpClientHelper.wrapException(String.format("Query request failed (Attempt: %s/%s - Duration: %s)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,12 @@ public interface ClickHouseBinaryFormatReader extends AutoCloseable {
boolean hasNext();

/**
* Moves cursor to the next row. Must be called before reading the first row.
* Moves cursor to the next row. Must be called before reading the first row. Returns reference to
* an internal record representation. It means that next call to the method will affect value in returned Map.
* This is done for memory usage optimization.
* Method is intended to be used only by the client not an application.
*
* @return map filled with column values or null if no more records are available
* @return reference to a map filled with column values or null if no more records are available
*/
Map<String, Object> next();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,15 @@ public long getResultRows() {
return operationMetrics.getMetric(ServerMetrics.RESULT_ROWS).getLong();
}


/**
* Alias for {@link ServerMetrics#TOTAL_ROWS_TO_READ}
* @return estimated number of rows to read
*/
public long getTotalRowsToRead() {
return operationMetrics.getMetric(ServerMetrics.TOTAL_ROWS_TO_READ).getLong();
}

/**
* Alias for {@link OperationMetrics#getQueryId()}
* @return query id of the request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public class SamplePOJO {
private ClickHouseBitmap groupBitmapUint32;
private ClickHouseBitmap groupBitmapUint64;

private String keyword;

public SamplePOJO() {
final Random random = new Random();
byteValue = (byte) random.nextInt();
Expand Down Expand Up @@ -180,6 +182,8 @@ public SamplePOJO() {

groupBitmapUint32 = ClickHouseBitmap.wrap(random.ints(5, Integer.MAX_VALUE - 100, Integer.MAX_VALUE).toArray());
groupBitmapUint64 = ClickHouseBitmap.wrap(random.longs(5, Long.MAX_VALUE - 100, Long.MAX_VALUE).toArray());

keyword = "database";
}

public byte getByteValue() {
Expand Down Expand Up @@ -574,6 +578,14 @@ public void setGroupBitmapUint64(ClickHouseBitmap groupBitmapUint64) {
this.groupBitmapUint64 = groupBitmapUint64;
}

public String getKeyword() {
return keyword;
}

public void setKeyword(String keyword) {
this.keyword = keyword;
}

@Override
public String toString() {
return "SamplePOJO{" +
Expand Down Expand Up @@ -683,7 +695,8 @@ public static String generateTableCreateSQL(String tableName) {
"nested Nested (innerInt Int32, innerString String, " +
"innerNullableInt Nullable(Int32)), " +
"groupBitmapUint32 AggregateFunction(groupBitmap, UInt32), " +
"groupBitmapUint64 AggregateFunction(groupBitmap, UInt64) " +
"groupBitmapUint64 AggregateFunction(groupBitmap, UInt64), " +
"keyword LowCardinality(String) " +
") ENGINE = MergeTree ORDER BY ()";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.MappingIterator;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.io.BaseEncoding;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringEscapeUtils;
import org.testng.Assert;
Expand All @@ -55,13 +56,15 @@
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -486,7 +489,8 @@ record = reader.next();
"col1 Array(UInt32)",
"col2 Array(Array(Int32))",
"col3 Array(UInt64)",
"col4 Array(Bool)"
"col4 Array(Bool)",
"col5 Array(String)"
);

private final static List<Function<String, Object>> ARRAY_VALUE_GENERATORS = Arrays.asList(
Expand All @@ -505,8 +509,17 @@ record = reader.next();
RANDOM.longs(10, 0, Long.MAX_VALUE)
.mapToObj(BigInteger::valueOf).collect(Collectors.toList()),
c -> RANDOM.ints(10, 0, 1)
.mapToObj(i -> i == 0 ).collect(Collectors.toList())

.mapToObj(i -> i == 0 ).collect(Collectors.toList()),
c -> {
UUID uuid = UUID.randomUUID();
byte[] bts = ByteBuffer.allocate(16)
.putLong(uuid.getMostSignificantBits())
.putLong(uuid.getLeastSignificantBits())
.array();
String sep = "\\x";
String hex = sep + BaseEncoding.base16().withSeparator(sep, 2).encode(bts);
return Arrays.asList(hex);
}
);

@Test(groups = {"integration"})
Expand Down Expand Up @@ -546,6 +559,7 @@ public void testArrayValues() throws Exception {
Assert.assertEquals(col4Values, data.get(0).get("col4"));
boolean[] col4Array = reader.getBooleanArray("col4");
Assert.assertEquals(col4Array, ((List)data.get(0).get("col4")).toArray());
Assert.assertEquals(reader.getList("col5"), ((List)data.get(0).get("col5")));
}

@Test
Expand Down Expand Up @@ -1452,6 +1466,13 @@ private Map<String, Object> writeValuesRow(StringBuilder insertStmtBuilder, List
}
insertStmtBuilder.setLength(insertStmtBuilder.length() - 2);
insertStmtBuilder.append("}, ");
} else if (value instanceof List) {
insertStmtBuilder.append("[");
for (Object item : (List)value) {
insertStmtBuilder.append(quoteValue(item)).append(", ");
}
insertStmtBuilder.setLength(insertStmtBuilder.length() - 2);
insertStmtBuilder.append("], ");
} else {
insertStmtBuilder.append(value).append(", ");
}
Expand All @@ -1463,7 +1484,9 @@ private Map<String, Object> writeValuesRow(StringBuilder insertStmtBuilder, List

private String quoteValue(Object value) {
if (value instanceof String) {
return '\'' + value.toString() + '\'';
String strVal = (String)value;

return '\'' + strVal.replaceAll("\\\\", "\\\\\\\\") + '\'';
}
return value.toString();
}
Expand Down Expand Up @@ -2000,4 +2023,37 @@ public void testServerTimezone() throws Exception {
Assert.assertEquals(serverUtcTime.withZoneSameInstant(ZoneId.of("America/New_York")), serverEstTime);
}
}

@Test(groups = {"integration"})
public void testLowCardinalityValues() throws Exception {
final String table = "test_low_cardinality_values";
final String tableCreate = "CREATE TABLE " + table + "(rowID Int32, keyword LowCardinality(String)) Engine = MergeTree ORDER BY ()";

client.execute("DROP TABLE IF EXISTS " + table);
client.execute(tableCreate);

client.execute("INSERT INTO " + table + " VALUES (0, 'db'), (1, 'fast'), (2, 'not a java')");
String[] values = new String[] {"db", "fast", "not a java"};
Collection<GenericRecord> records = client.queryAll("SELECT * FROM " + table);
for (GenericRecord record : records) {
int rowId = record.getInteger("rowID");
Assert.assertEquals(record.getString("keyword"), values[rowId]);
}
}

@Test(groups = {"integration"})
public void testGettingRowsBeforeLimit() throws Exception {
int expectedTotalRowsToRead = 100;
List<GenericRecord> serverVersion = client.queryAll("SELECT version()");
if (ClickHouseVersion.of(serverVersion.get(0).getString(1)).check("(,23.8]")) {
// issue in prev. release.
expectedTotalRowsToRead = 0;
}

try (QueryResponse response = client.query("SELECT number FROM system.numbers LIMIT 100").get()) {
Assert.assertTrue(response.getResultRows() < 1000);

Assert.assertEquals(response.getTotalRowsToRead(), expectedTotalRowsToRead);
}
}
}
15 changes: 14 additions & 1 deletion jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL
} else {
response = connection.client.query(lastSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS);
}

if (response.getFormat().isText()) {
throw new SQLException("Only RowBinaryWithNameAndTypes is supported for output format. Please check your query.",
ExceptionUtils.SQL_STATE_CLIENT_ERROR);
}
ClickHouseBinaryFormatReader reader = connection.client.newBinaryFormatReader(response);

currentResultSet = new ResultSetImpl(this, response, reader);
Expand Down Expand Up @@ -225,7 +230,7 @@ public int executeUpdate(String sql, QuerySettings settings) throws SQLException
try (QueryResponse response = queryTimeout == 0 ? connection.client.query(lastSql, mergedSettings).get()
: connection.client.query(lastSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS)) {
currentResultSet = null;
updateCount = (int) response.getWrittenRows();
updateCount = Math.max(0, (int) response.getWrittenRows()); // when statement alters schema no result rows returned.
metrics = response.getMetrics();
lastQueryId = response.getQueryId();
} catch (Exception e) {
Expand Down Expand Up @@ -601,4 +606,12 @@ public String enquoteNCharLiteral(String val) throws SQLException {
checkClosed();
return Statement.super.enquoteNCharLiteral(val);
}

/**
* Return query ID of last executed statement. It is not guaranteed when statements is used concurrently.
* @return query ID
*/
public String getLastQueryId() {
return lastQueryId;
}
}
13 changes: 6 additions & 7 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/DataTypeTests.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.clickhouse.jdbc;

import com.clickhouse.client.api.ClientConfigProperties;
import com.clickhouse.data.Tuple;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -11,7 +10,6 @@
import java.math.BigInteger;
import java.sql.Connection;
import java.sql.Date;
import java.sql.DriverManager;
import java.sql.JDBCType;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
Expand All @@ -26,7 +24,6 @@
import java.util.GregorianCalendar;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.Random;
import java.util.TimeZone;
import java.util.UUID;
Expand Down Expand Up @@ -318,7 +315,8 @@ public void testStringTypes() throws SQLException {
runQuery("CREATE TABLE test_strings (order Int8, "
+ "str String, fixed FixedString(6), "
+ "enum Enum8('a' = 6, 'b' = 7, 'c' = 8), enum8 Enum8('a' = 1, 'b' = 2, 'c' = 3), enum16 Enum16('a' = 1, 'b' = 2, 'c' = 3), "
+ "uuid UUID, ipv4 IPv4, ipv6 IPv6"
+ "uuid UUID, ipv4 IPv4, ipv6 IPv6, "
+ "escaped String "
+ ") ENGINE = MergeTree ORDER BY ()");

// Insert random (valid) values
Expand All @@ -333,10 +331,10 @@ public void testStringTypes() throws SQLException {
String uuid = UUID.randomUUID().toString();
String ipv4 = rand.nextInt(256) + "." + rand.nextInt(256) + "." + rand.nextInt(256) + "." + rand.nextInt(256);
String ipv6 = "2001:adb8:85a3:1:2:8a2e:370:7334";

String escaped = "\\xA3\\xA3\\x12\\xA0\\xDF\\x13\\x4E\\x8C\\x87\\x74\\xD4\\x53\\xDB\\xFC\\x34\\x95";

try (Connection conn = getConnection()) {
try (PreparedStatement stmt = conn.prepareStatement("INSERT INTO test_strings VALUES ( 1, ?, ?, ?, ?, ?, ?, ?, ? )")) {
try (PreparedStatement stmt = conn.prepareStatement("INSERT INTO test_strings VALUES ( 1, ?, ?, ?, ?, ?, ?, ?, ?, ? )")) {
stmt.setString(1, str);
stmt.setString(2, fixed);
stmt.setString(3, enum8);
Expand All @@ -345,6 +343,7 @@ public void testStringTypes() throws SQLException {
stmt.setString(6, uuid);
stmt.setString(7, ipv4);
stmt.setString(8, ipv6);
stmt.setString(9, escaped);
stmt.executeUpdate();
}
}
Expand All @@ -365,7 +364,7 @@ public void testStringTypes() throws SQLException {
assertEquals(rs.getString("uuid"), uuid);
assertEquals(rs.getString("ipv4"), "/" + ipv4);
assertEquals(rs.getString("ipv6"), "/" + ipv6);

assertEquals(rs.getString("escaped"), escaped);
assertFalse(rs.next());
}
}
Expand Down
9 changes: 9 additions & 0 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public void testExecuteQuerySimpleNumbers() throws Exception {
assertEquals(rs.getLong("num"), 1);
assertFalse(rs.next());
}
Assert.assertFalse(((StatementImpl)stmt).getLastQueryId().isEmpty());
}
}
}
Expand Down Expand Up @@ -530,4 +531,12 @@ public void testConcurrentCancel() throws Exception {
}
}
}

@Test(groups = {"integration"})
public void testTextFormatInResponse() throws Exception {
try (Connection conn = getJdbcConnection();
Statement stmt = conn.createStatement()) {
Assert.expectThrows(SQLException.class, () ->stmt.executeQuery("SELECT 1 FORMAT JSON"));
}
}
}

0 comments on commit 93dfb96

Please sign in to comment.