[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9832: [multistage] support sort push-down

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org