Skip to content
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

[client-v2] Fixes handling empty results while using reader #1845

Merged
merged 1 commit into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -148,11 +151,16 @@ public <T> T readValue(String colName) {

@Override
public boolean hasNext() {
if (initialState) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have no results? IE can we be sure it SHOULD always be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is possible to have no results: empty dataset or completely no data in a response body.
Both cases are handled. The initialState is changed once any bytes are read from input stream).

readNextRecord();
}

return hasNext;
}


protected void readNextRecord() {
initialState = false;
try {
nextRecordEmpty.set(true);
if (!readRecord(nextRecord)) {
Expand Down Expand Up @@ -195,6 +203,7 @@ public Map<String, Object> next() {
}

protected void endReached() {
initialState = false;
hasNext = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down
Loading