From a8ba8a73a86cc6e1d5d4692ce4068bd3f3a2730a Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Mon, 30 Sep 2024 10:43:12 -0700 Subject: [PATCH] fixes handling empty results while using reader --- .../internal/AbstractBinaryFormatReader.java | 9 ++++ .../clickhouse/client/api/query/Records.java | 4 +- .../clickhouse/client/query/QueryTests.java | 53 +++++++++++++++++-- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java index e4c3541f4..1f4b741bd 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java @@ -54,6 +54,9 @@ public abstract class AbstractBinaryFormatReader implements ClickHouseBinaryForm private volatile boolean hasNext = true; + + private volatile boolean initialState = true; // reader is in initial state, no records have been read yet + protected AbstractBinaryFormatReader(InputStream inputStream, QuerySettings querySettings, TableSchema schema, BinaryStreamReader.ByteBufferAllocator byteBufferAllocator) { this.input = inputStream; @@ -148,11 +151,16 @@ public T readValue(String colName) { @Override public boolean hasNext() { + if (initialState) { + readNextRecord(); + } + return hasNext; } protected void readNextRecord() { + initialState = false; try { nextRecordEmpty.set(true); if (!readRecord(nextRecord)) { @@ -195,6 +203,7 @@ public Map next() { } protected void endReached() { + initialState = false; hasNext = false; } diff --git a/client-v2/src/main/java/com/clickhouse/client/api/query/Records.java b/client-v2/src/main/java/com/clickhouse/client/api/query/Records.java index 4df1d9387..46d0e827d 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/query/Records.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/query/Records.java @@ -55,10 +55,12 @@ public Spliterator spliterator() { /** * Returns {@code true} if this collection contains no elements. + * Prefer this method over {@link #getResultRows()} == 0 because current method reflect actual state of the collection + * while {@link #getResultRows()} is send from server before sending actual data. * * @return {@code true} if this collection contains no elements */ - boolean isEmpty() { + public boolean isEmpty() { return empty; } diff --git a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java index 78cf6cf29..355350058 100644 --- a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java @@ -261,14 +261,25 @@ public void testReadRecordsGetFirstRecord() throws Exception { Assert.assertFalse(iter.hasNext()); } - @Test(groups = {"integration"}) - public void testReadRecordsNoResult() throws Exception { - Records records = client.queryRecords("CREATE DATABASE IF NOT EXISTS test_db").get(3, TimeUnit.SECONDS); + @Test(description = "Verifies correct handling of empty data set while column information is present", groups = {"integration"}) + public void testQueryRecordsOnEmptyDataset() throws Exception { + Records records = client.queryRecords("SELECT 1 LIMIT 0").get(3, TimeUnit.SECONDS); Iterator iter = records.iterator(); Assert.assertFalse(iter.hasNext()); } + @Test(description = "Verifies correct handling when no column information expected", groups = {"integration"}) + public void testQueryRecordsWithEmptyResult() throws Exception { + // This test uses a query that returns no data and no column information + try (Records records = client.queryRecords("CREATE DATABASE IF NOT EXISTS test_db").get(3, TimeUnit.SECONDS)) { + Assert.assertTrue(records.isEmpty()); + for (GenericRecord record : records) { + Assert.fail("unexpected record: " + record); + } + } + } + @Test(groups = {"integration"}) public void testQueryAll() throws Exception { List> dataset = prepareDataSet(DATASET_TABLE, DATASET_COLUMNS, DATASET_VALUE_GENERATORS, 10); @@ -298,7 +309,8 @@ public void testQueryAllSimple(int numberOfRecords) throws Exception { @Test(groups = {"integration"}) public void testQueryAllNoResult() throws Exception { - List records = client.queryAll("CREATE DATABASE IF NOT EXISTS test_db"); + List records = client.queryAll("SELECT 1 LIMIT 0"); + Assert.assertEquals(records.size(), 0); Assert.assertTrue(records.isEmpty()); } @@ -457,6 +469,14 @@ record = reader.next(); } ); + @Test(groups = {"integration"}) + public void testBinaryReaderOnQueryWithNoResult() throws Exception { + try (QueryResponse response = client.query("SELECT 1 LIMIT 0").get(3, TimeUnit.SECONDS)) { + ClickHouseBinaryFormatReader reader = client.newBinaryFormatReader(response); + Assert.assertFalse(reader.hasNext()); + Assert.assertNull(reader.next()); + } + } @Test(groups = {"integration"}) public void testArrayValues() throws Exception { @@ -539,6 +559,31 @@ public void testQueryExceptionHandling() throws Exception { } } + @Test + public void testQueryRecordsEmptyResult() throws Exception { + try (Records records = client.queryRecords("SELECT 1 LIMIT 0").get(3, TimeUnit.SECONDS)) { + Assert.assertTrue(records.isEmpty()); + for (GenericRecord record : records) { + Assert.fail("unexpected record: " + record); + } + } + } + + @Test(description = "Verifies that queryRecords reads all values from the response", groups = {"integration"}) + public void testQueryRecordsReadsAllValues() throws Exception { + try (Records records = client.queryRecords("SELECT toInt32(number) FROM system.numbers LIMIT 3").get(3, TimeUnit.SECONDS)) { + Assert.assertFalse(records.isEmpty()); + Assert.assertEquals(records.getResultRows(), 3); + + int expectedNumber = 0; + for (GenericRecord record : records) { + Assert.assertEquals(record.getInteger(1), expectedNumber); + expectedNumber++; + } + + Assert.assertEquals(expectedNumber, 3); + } + } private final static List NULL_DATASET_COLUMNS = Arrays.asList( "id UInt32",