drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [drill] vvysotskyi commented on a change in pull request #2189: DRILL-7877 - Streaming REST API Fails to Send Multiple Batches
Date Wed, 17 Mar 2021 18:02:19 GMT

vvysotskyi commented on a change in pull request #2189:
URL: https://github.com/apache/drill/pull/2189#discussion_r596263674



##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/stream/StreamingHttpConnection.java
##########
@@ -104,11 +104,9 @@ public void outputAvailable(OutputStream out) throws IOException {
   public void sendData(RpcOutcomeListener<Ack> listener, QueryDataPackage data) {
     VectorContainer batch = data.batch();
     try {
-      if (batchCount == 0) {
-        batchHolder = new BatchHolder(batch);
-        reader = new PushResultSetReaderImpl(batchHolder);
-        startSignal.await();
-      }
+      batchHolder = new BatchHolder(batch);
+      reader = new PushResultSetReaderImpl(batchHolder);
+      startSignal.await();

Review comment:
       The actual reason why this issue appears is because of `ReaderIndex.rowCount` initialized
on the first batch with 0 value and this value is left for the next batches. The ideal fix
would be to enhance the logic to update `ReaderIndex.rowCount` when calling `RowSetReaderImpl.newBatch()`.
But I don't see significant issues with an existing solution.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message