Merge pull request #1845 from ClickHouse/clientv2_fix_emptyresponse_e…

…rror

fixes handling empty results while using reader

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> 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<String, Object> next() {
     }
 
     protected void endReached() {
+        initialState = false;
         hasNext = false;
     }
 

client-v2/src/main/java/com/clickhouse/client/api/query/Records.java

@@ -55,10 +55,12 @@ public Spliterator<GenericRecord> 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;
     }
 

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<GenericRecord> 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<Map<String, Object>> 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<GenericRecord> records = client.queryAll("CREATE DATABASE IF NOT EXISTS test_db");
+        List<GenericRecord> 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<String> NULL_DATASET_COLUMNS = Arrays.asList(
             "id UInt32",