justinborromeo commented on a change in pull request #7024: Time Ordering 
Option on Small-Result-Set Scan Queries
URL: https://github.com/apache/incubator-druid/pull/7024#discussion_r257435395
 
 

 ##########
 File path: 
processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java
 ##########
 @@ -58,37 +68,64 @@ public ScanQueryQueryToolChest(
   @Override
   public QueryRunner<ScanResultValue> mergeResults(final 
QueryRunner<ScanResultValue> runner)
   {
-    return new QueryRunner<ScanResultValue>()
-    {
-      @Override
-      public Sequence<ScanResultValue> run(
-          final QueryPlus<ScanResultValue> queryPlus, final Map<String, 
Object> responseContext
-      )
-      {
-        // Ensure "legacy" is a non-null value, such that all other nodes this 
query is forwarded to will treat it
-        // the same way, even if they have different default legacy values.
-        final ScanQuery scanQuery = ((ScanQuery) 
queryPlus.getQuery()).withNonNullLegacy(scanQueryConfig);
-        final QueryPlus<ScanResultValue> queryPlusWithNonNullLegacy = 
queryPlus.withQuery(scanQuery);
+    return (queryPlus, responseContext) -> {
+      // Ensure "legacy" is a non-null value, such that all other nodes this 
query is forwarded to will treat it
+      // the same way, even if they have different default legacy values.
+      final ScanQuery scanQuery = ((ScanQuery) 
queryPlus.getQuery()).withNonNullLegacy(scanQueryConfig);
+      final QueryPlus<ScanResultValue> queryPlusWithNonNullLegacy = 
queryPlus.withQuery(scanQuery);
+      final BaseSequence.IteratorMaker scanQueryLimitRowIteratorMaker =
+          new BaseSequence.IteratorMaker<ScanResultValue, 
ScanQueryLimitRowIterator>()
+          {
+            @Override
+            public ScanQueryLimitRowIterator make()
+            {
+              return new ScanQueryLimitRowIterator(runner, 
queryPlusWithNonNullLegacy, responseContext);
+            }
 
+            @Override
+            public void cleanup(ScanQueryLimitRowIterator iterFromMake)
+            {
+              CloseQuietly.close(iterFromMake);
+            }
+          };
+
+      if (scanQuery.getTimeOrder().equals(ScanQuery.TIME_ORDER_NONE)) {
         if (scanQuery.getLimit() == Long.MAX_VALUE) {
           return runner.run(queryPlusWithNonNullLegacy, responseContext);
         }
-        return new BaseSequence<>(
-            new BaseSequence.IteratorMaker<ScanResultValue, 
ScanQueryLimitRowIterator>()
+        return new BaseSequence<ScanResultValue, 
ScanQueryLimitRowIterator>(scanQueryLimitRowIteratorMaker);
+      } else if 
((scanQuery.getTimeOrder().equals(ScanQuery.TIME_ORDER_ASCENDING) ||
+                  
scanQuery.getTimeOrder().equals(ScanQuery.TIME_ORDER_DESCENDING))
+                 && scanQuery.getLimit() <= 
scanQueryConfig.getMaxRowsTimeOrderedInMemory()) {
+        Iterator<ScanResultValue> scanResultIterator = 
scanQueryLimitRowIteratorMaker.make();
 
 Review comment:
   Ah shoot, I misunderstood part of the issue.  I wrote this under the 
assumption that it would act like an ORDER BY operator.  So if I understand 
correctly now, this query:
   
   ```
   {
     ...
     "timeOrdering":"descending" 
     "limit":100
   }
   ```
   
   should return the latest 100 rows?
   
   Conversely, this query:
   
   ```
   {
     ...
     "timeOrdering":"ascending" 
     "limit":100
   }
   ```
   
   should return the earliest 100 rows?
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to