[GitHub] [drill] paul-rogers commented on a diff in pull request #2728: DRILL-8372: Unfreed buffers when running a LIMIT 0 query over delimited text

["paul-rogers (via GitHub)" <gi...@apache.org>]

paul-rogers commented on code in PR #2728:
URL: https://github.com/apache/drill/pull/2728#discussion_r1087483902


##########
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java:
##########
@@ -75,7 +75,7 @@ public IterOutcome innerNext() {
         upStream = next(incoming);
       }
       // If EMIT that means leaf operator is UNNEST, in this case refresh the limit states and return EMIT.
-      if (upStream == EMIT) {
+      if (upStream == EMIT || upStream == NONE) {

Review Comment:
   This doesn't seem to be quite the right solution. This block of code is for a very particular case: an UNNEST.
   
   Expanding this code, look at the top of the loop:
   
   ```java
       if (!first && !needMoreRecords(numberOfRecords)) {
   ```
   
   With a LIMIT 0, we hit the limit on the first batch. I'm not quite sure why the `!first` is in place. Maybe history would tell us. Perhaps the right answer is something like:
   
   ```java
   if ( !needMoreRecords(numberOfRecords)) {
        outgoingSv.setRecordCount(0);
        VectorAccessibleUtilities.clear(incoming);
        return super.innerNext();
   }
   if (!first) {
     ...
   ```
   
   I suspect that the logic actually needs more analysis. What does it do on the first batch now? What does `super.innerNext()` do, and do we want that if we've reached the limit?
   
   Generally, the debugger is the best way to sort this out. Try a LIMIT 0, a LIMIT n where n < size of the first batch, LIMIT n where n > batch size && n < 2 * batch size, etc.



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org

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