Jackie-Jiang commented on code in PR #9832:
URL: https://github.com/apache/pinot/pull/9832#discussion_r1035078758


##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorService.java:
##########
@@ -129,36 +128,23 @@ public void reduceWithOrdering(Collection<DataTable> 
dataTables, boolean nullHan
    * <p>Should be called after method "reduceWithOrdering()".
    */
   public ResultTable renderResultTableWithOrdering() {
-    int[] columnIndices = 
SelectionOperatorUtils.getColumnIndices(_selectionColumns, _dataSchema);
-    int numColumns = columnIndices.length;
-
-    // Construct the result data schema
-    String[] columnNames = _dataSchema.getColumnNames();
-    ColumnDataType[] columnDataTypes = _dataSchema.getColumnDataTypes();
-    String[] resultColumnNames = new String[numColumns];
-    ColumnDataType[] resultColumnDataTypes = new ColumnDataType[numColumns];
-    for (int i = 0; i < numColumns; i++) {
-      int columnIndex = columnIndices[i];
-      resultColumnNames[i] = columnNames[columnIndex];
-      resultColumnDataTypes[i] = columnDataTypes[columnIndex];
-    }
-    DataSchema resultDataSchema = new DataSchema(resultColumnNames, 
resultColumnDataTypes);
+    Iterator<Object[]> rows = new Iterator<Object[]>() {
+      @Override
+      public boolean hasNext() {
+        return _rows.size() > _offset;
+      }
 
-    // Extract the result rows
-    LinkedList<Object[]> rowsInSelectionResults = new LinkedList<>();
-    while (_rows.size() > _offset) {
-      Object[] row = _rows.poll();
-      assert row != null;
-      Object[] extractedRow = new Object[numColumns];
-      for (int i = 0; i < numColumns; i++) {
-        Object value = row[columnIndices[i]];
-        if (value != null) {
-          extractedRow[i] = resultColumnDataTypes[i].convertAndFormat(value);
-        }
+      @Override
+      public Object[] next() {
+        return _rows.poll();
       }
-      rowsInSelectionResults.addFirst(extractedRow);
-    }
+    };
 
-    return new ResultTable(resultDataSchema, rowsInSelectionResults);
+    return SelectionOperatorUtils.arrangeColumnsToMatchProjection(

Review Comment:
   (format) Reformat this to match the [Pinot 
Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#setup-ide)



##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java:
##########
@@ -647,4 +649,42 @@ public static <T> void addToPriorityQueue(T value, 
PriorityQueue<T> queue, int m
       queue.offer(value);
     }
   }
+
+  public static <T> T arrangeColumnsToMatchProjection(DataSchema dataSchema, 
Iterator<Object[]> rows,

Review Comment:
   Suggest not extracting the whole method because it is not as readable, and 
not very reusable. It is like forcefully stitching 2 methods together, with 
potential performance overhead of using iterator, passing function and extra 
per-record if check.
   We can extract the part of creating `resultDataSchema`: `public static 
DataSchema arrangeColumns(DataSchema dataSchema, int[] columnIndices)`



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