jt-kloudfuse commented on a change in pull request #8228:
URL: https://github.com/apache/pinot/pull/8228#discussion_r813372991



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
##########
@@ -180,13 +182,62 @@ public String toExplainString() {
 
   @Override
   protected IntermediateResultsBlock getNextBlock() {
-    if (_expressions.size() == _orderByExpressions.size()) {
+    if (_allOrderByColsPreSorted) {
+      return computeAllPreSorted();
+    } else if (_expressions.size() == _orderByExpressions.size()) {
       return computeAllOrdered();
     } else {
       return computePartiallyOrdered();
     }
   }
 
+  private IntermediateResultsBlock computeAllPreSorted() {
+    int numExpressions = _expressions.size();
+
+    // Fetch all the expressions and insert them into the priority queue
+    BlockValSet[] blockValSets = new BlockValSet[numExpressions];
+    int numColumnsProjected = _transformOperator.getNumColumnsProjected();
+    TransformBlock transformBlock;
+    while (_numDocsScanned < _numRowsToKeep && (transformBlock = 
_transformOperator.nextBlock()) != null) {
+      for (int i = 0; i < numExpressions; i++) {
+        ExpressionContext expression = _expressions.get(i);
+        blockValSets[i] = transformBlock.getBlockValueSet(expression);
+      }
+      RowBasedBlockValueFetcher blockValueFetcher = new 
RowBasedBlockValueFetcher(blockValSets);
+      int numDocsFetched = transformBlock.getNumDocs();
+      for (int i = 0; i < numDocsFetched && (_numDocsScanned < 
_numRowsToKeep); i++) {
+        SelectionOperatorUtils.addToPriorityQueue(blockValueFetcher.getRow(i), 
_rows, _numRowsToKeep);
+        _numDocsScanned++;
+      }
+    }
+    _numEntriesScannedPostFilter += _numDocsScanned * numColumnsProjected;
+
+    // Create the data schema
+    String[] columnNames = new String[numExpressions];
+    DataSchema.ColumnDataType[] columnDataTypes = new 
DataSchema.ColumnDataType[numExpressions];
+    for (int i = 0; i < numExpressions; i++) {
+      columnNames[i] = _expressions.get(i).toString();
+    }
+
+    int numOrderByExpressions = _orderByExpressions.size();
+    for (int i = 0; i < numOrderByExpressions; i++) {
+      TransformResultMetadata expressionMetadata = 
_orderByExpressionMetadata[i];
+      columnDataTypes[i] =
+          
DataSchema.ColumnDataType.fromDataType(expressionMetadata.getDataType(), 
expressionMetadata.isSingleValue());
+    }
+
+    List<ExpressionContext> nonOrderByExpressions = 
_expressions.subList(numOrderByExpressions, numExpressions);
+    int numNonOrderByExpressions = nonOrderByExpressions.size();
+    for (int i = 0; i < numNonOrderByExpressions; i++) {
+      TransformResultMetadata expressionMetadata = 
_transformOperator.getResultMetadata(nonOrderByExpressions.get(i));
+      columnDataTypes[numOrderByExpressions + i] =
+          
DataSchema.ColumnDataType.fromDataType(expressionMetadata.getDataType(), 
expressionMetadata.isSingleValue());
+    }

Review comment:
       I am fine with either.
   Basically, we wont be using the _orderByExpressionMetadata that we created 
in the constructor but making getResultMetadata calls again for orderBy columns.
   
   Fwiw, its not an extra list per se.. because subList creates a view on an 
existing list.
   




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



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

Reply via email to