You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2019/09/04 05:07:31 UTC

[incubator-pinot] 01/01: Refactor TransformBlockDataFetcher so that DISTINCT and reuse it

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch transform_fetcher
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit e0ef1a0547feb8f8794f1e0453227856c1660eed
Author: Jackie (Xiaotian) Jiang <xa...@linkedin.com>
AuthorDate: Tue Sep 3 22:02:40 2019 -0700

    Refactor TransformBlockDataFetcher so that DISTINCT and reuse it
    
    1. Refactor TransformBlockDataFetcher to take only the BlockValSets
    2. Remove the redundant dictionary handling logic (already handled in DataFetcher)
    3. Refactor SelectionOperator to simplify the logic, and only parse selection columns once
    4. Move the sort columns (expressions) into the front of the data schema
---
 .../apache/pinot/core/common/BaseBlockValSet.java  |   9 +-
 .../org/apache/pinot/core/common/BlockValSet.java  |   2 +
 .../core/operator/blocks/ProjectionBlock.java      |   9 +-
 .../core/operator/docvalsets/MultiValueSet.java    |   5 +
 .../operator/docvalsets/ProjectionBlockValSet.java |  23 +-
 .../core/operator/docvalsets/SingleValueSet.java   |   5 +
 .../operator/docvalsets/TransformBlockValSet.java  |   9 +-
 .../core/operator/query/SelectionOperator.java     | 279 +++++++--------
 .../transform/TransformBlockDataFetcher.java       | 373 +++++++--------------
 .../core/operator/transform/TransformOperator.java |  17 +-
 .../apache/pinot/core/plan/SelectionPlanNode.java  |   3 +-
 .../apache/pinot/core/plan/TransformPlanNode.java  |  98 +++---
 12 files changed, 336 insertions(+), 496 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/BaseBlockValSet.java b/pinot-core/src/main/java/org/apache/pinot/core/common/BaseBlockValSet.java
index 88f2579..252a4f0 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/common/BaseBlockValSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/common/BaseBlockValSet.java
@@ -18,20 +18,13 @@
  */
 package org.apache.pinot.core.common;
 
-import org.apache.pinot.common.data.FieldSpec;
-
-
 /**
  * Abstract base class implementation for BlockValSet
  */
 public abstract class BaseBlockValSet implements BlockValSet {
-  @Override
-  public BlockValIterator iterator() {
-    throw new UnsupportedOperationException();
-  }
 
   @Override
-  public FieldSpec.DataType getValueType() {
+  public BlockValIterator iterator() {
     throw new UnsupportedOperationException();
   }
 
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java b/pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java
index b39d126..98891e1 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java
@@ -27,6 +27,8 @@ public interface BlockValSet {
 
   DataType getValueType();
 
+  boolean isSingleValue();
+
   /**
    * DOCUMENT ID BASED APIs
    */
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java
index 48dda48..1179885 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java
@@ -63,13 +63,10 @@ public class ProjectionBlock implements Block {
     throw new UnsupportedOperationException();
   }
 
-  // TODO: let selection operator use DataBlockCache as well
-  public Block getBlock(String column) {
-    return _blockMap.get(column);
-  }
-
   public BlockValSet getBlockValueSet(String column) {
-    return new ProjectionBlockValSet(_dataBlockCache, column, _blockMap.get(column).getMetadata().getDataType());
+    BlockMetadata blockMetadata = _blockMap.get(column).getMetadata();
+    return new ProjectionBlockValSet(_dataBlockCache, column, blockMetadata.getDataType(),
+        blockMetadata.isSingleValue());
   }
 
   public DocIdSetBlock getDocIdSetBlock() {
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/MultiValueSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/MultiValueSet.java
index 37cc021..cde916a 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/MultiValueSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/MultiValueSet.java
@@ -45,4 +45,9 @@ public final class MultiValueSet extends BaseBlockValSet {
   public DataType getValueType() {
     return _dataType;
   }
+
+  @Override
+  public boolean isSingleValue() {
+    return false;
+  }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/ProjectionBlockValSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/ProjectionBlockValSet.java
index ac9f89e..cce946f 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/ProjectionBlockValSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/ProjectionBlockValSet.java
@@ -18,7 +18,7 @@
  */
 package org.apache.pinot.core.operator.docvalsets;
 
-import org.apache.pinot.common.data.FieldSpec;
+import org.apache.pinot.common.data.FieldSpec.DataType;
 import org.apache.pinot.core.common.BaseBlockValSet;
 import org.apache.pinot.core.common.DataBlockCache;
 import org.apache.pinot.core.operator.ProjectionOperator;
@@ -32,7 +32,8 @@ import org.apache.pinot.core.operator.ProjectionOperator;
 public class ProjectionBlockValSet extends BaseBlockValSet {
   private final DataBlockCache _dataBlockCache;
   private final String _column;
-  private final FieldSpec.DataType _dataType;
+  private final DataType _dataType;
+  private final boolean _singleValue;
 
   /**
    * Constructor for the class.
@@ -42,10 +43,21 @@ public class ProjectionBlockValSet extends BaseBlockValSet {
    * @param dataBlockCache data block cache
    * @param column Projection column.
    */
-  public ProjectionBlockValSet(DataBlockCache dataBlockCache, String column, FieldSpec.DataType dataType) {
+  public ProjectionBlockValSet(DataBlockCache dataBlockCache, String column, DataType dataType, boolean singleValue) {
     _dataBlockCache = dataBlockCache;
     _column = column;
     _dataType = dataType;
+    _singleValue = singleValue;
+  }
+
+  @Override
+  public DataType getValueType() {
+    return _dataType;
+  }
+
+  @Override
+  public boolean isSingleValue() {
+    return _singleValue;
   }
 
   @Override
@@ -116,11 +128,6 @@ public class ProjectionBlockValSet extends BaseBlockValSet {
   }
 
   @Override
-  public FieldSpec.DataType getValueType() {
-    return _dataType;
-  }
-
-  @Override
   public int[] getDictionaryIdsSV() {
     return _dataBlockCache.getDictIdsForSVColumn(_column);
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/SingleValueSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/SingleValueSet.java
index ea47430..9fa04ab 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/SingleValueSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/SingleValueSet.java
@@ -49,6 +49,11 @@ public final class SingleValueSet extends BaseBlockValSet {
   }
 
   @Override
+  public boolean isSingleValue() {
+    return true;
+  }
+
+  @Override
   public void getIntValues(int[] inDocIds, int inStartPos, int inDocIdsSize, int[] outValues, int outStartPos) {
     int inEndPos = inStartPos + inDocIdsSize;
     ReaderContext context = _reader.createContext();
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
index b8b6b22..a546663 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
@@ -18,7 +18,7 @@
  */
 package org.apache.pinot.core.operator.docvalsets;
 
-import org.apache.pinot.common.data.FieldSpec;
+import org.apache.pinot.common.data.FieldSpec.DataType;
 import org.apache.pinot.core.common.BaseBlockValSet;
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
@@ -43,11 +43,16 @@ public class TransformBlockValSet extends BaseBlockValSet {
   }
 
   @Override
-  public FieldSpec.DataType getValueType() {
+  public DataType getValueType() {
     return _transformFunction.getResultMetadata().getDataType();
   }
 
   @Override
+  public boolean isSingleValue() {
+    return _transformFunction.getResultMetadata().isSingleValue();
+  }
+
+  @Override
   public int[] getDictionaryIdsSV() {
     return _transformFunction.transformToDictIdsSV(_projectionBlock);
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOperator.java
index c20d194..0bd3e5f 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOperator.java
@@ -19,19 +19,16 @@
 package org.apache.pinot.core.operator.query;
 
 import java.io.Serializable;
-import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.PriorityQueue;
-import org.apache.pinot.common.data.FieldSpec.DataType;
 import org.apache.pinot.common.request.Selection;
 import org.apache.pinot.common.request.SelectionSort;
 import org.apache.pinot.common.request.transform.TransformExpressionTree;
 import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
 import org.apache.pinot.common.utils.primitive.ByteArray;
 import org.apache.pinot.core.common.BlockValSet;
 import org.apache.pinot.core.indexsegment.IndexSegment;
@@ -42,204 +39,160 @@ import org.apache.pinot.core.operator.blocks.TransformBlock;
 import org.apache.pinot.core.operator.transform.TransformBlockDataFetcher;
 import org.apache.pinot.core.operator.transform.TransformOperator;
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
-import org.apache.pinot.core.segment.index.readers.Dictionary;
 
 
-/**
- * This SelectionOnlyOperator will take care of applying a selection query to one IndexSegment.
- * nextBlock() will return an IntermediateResultBlock for the given IndexSegment.
- */
 public class SelectionOperator extends BaseOperator<IntermediateResultsBlock> {
-
-  private static final String OPERATOR_NAME = "SelectionOnlyOperator";
+  private static final String OPERATOR_NAME = "SelectionOperator";
 
   private final IndexSegment _indexSegment;
   private final TransformOperator _transformOperator;
-  private final DataSchema _dataSchema;
-  private final BlockValSet[] _blockValSets;
+  private final List<SelectionSort> _sortSequence;
+  private final List<TransformExpressionTree> _expressions;
   private final int _limit;
-  private Selection _selection;
-  private final int _offset;
   private final int _maxRows;
-  private final List<TransformExpressionTree> _expressions;
-  private final List<TransformExpressionTree> _selectExpressions;
-  private final List<TransformExpressionTree> _orderByExpressions;
-  private final List<Integer> _orderByIndices;
-  private final TransformResultMetadata[] _expressionResultMetadata;
-  private final Dictionary[] _dictionaries;
-  private Collection<Serializable[]> _rowEvents;
-  private PriorityQueue<Serializable[]> _priorityQueue;
+  private final Collection<Serializable[]> _rows;
+  private final PriorityQueue<Serializable[]> _priorityQueue;
+  private final BlockValSet[] _blockValSets;
+  private final DataSchema _dataSchema;
 
   private ExecutionStatistics _executionStatistics;
 
-  public SelectionOperator(IndexSegment indexSegment, Selection selection, TransformOperator transformOperator) {
+  /**
+   * NOTE: The expressions in the transform operator has the sort expressions at the front. The sort sequence is the
+   * deduplicated sort expressions.
+   */
+  public SelectionOperator(IndexSegment indexSegment, Selection selection, TransformOperator transformOperator,
+      List<SelectionSort> sortSequence) {
     _indexSegment = indexSegment;
-    _offset = selection.getOffset();
-    _limit = selection.getSize();
-    _selection = selection;
-    _maxRows = _offset + _limit;
     _transformOperator = transformOperator;
+    _sortSequence = sortSequence;
     _expressions = _transformOperator.getExpressions();
+    _limit = selection.getSize();
 
-    List<String> selectColumns = selection.getSelectionColumns();
-    if (selectColumns.size() == 1 && selectColumns.get(0).equals("*")) {
-      selectColumns = new LinkedList<>(indexSegment.getPhysicalColumnNames());
-    }
-
-    List<String> orderByColumns = new ArrayList<>();
-    if (selection.getSelectionSortSequence() != null) {
-      for (SelectionSort selectionSort : selection.getSelectionSortSequence()) {
-        String expression = selectionSort.getColumn();
-        orderByColumns.add(expression);
-      }
-    }
-    _selectExpressions = new ArrayList<>();
-    _orderByExpressions = new ArrayList<>();
-    _orderByIndices = new ArrayList<>();
-    for (int i = 0; i < _expressions.size(); i++) {
-      TransformExpressionTree expression = _expressions.get(i);
-      if (selectColumns.contains(expression.toString())) {
-        _selectExpressions.add(expression);
-      }
-    }
-
-    for (String orderByColumn : orderByColumns) {
-      for (int i = 0; i < _expressions.size(); i++) {
-        TransformExpressionTree expression = _expressions.get(i);
-        if (orderByColumn.equalsIgnoreCase(expression.toString())) {
-          _orderByExpressions.add(expression);
-          _orderByIndices.add(i);
-        }
-      }
+    boolean selectionOnly = sortSequence.isEmpty();
+    if (selectionOnly) {
+      // Selection only
+      _maxRows = _limit;
+      _rows = new ArrayList<>(_maxRows);
+      _priorityQueue = null;
+    } else {
+      // Selection order-by
+      _maxRows = selection.getOffset() + _limit;
+      _rows = null;
+      _priorityQueue = new PriorityQueue<>(_maxRows, getComparator());
     }
 
-    _blockValSets = new BlockValSet[_expressions.size()];
-    _expressionResultMetadata = new TransformResultMetadata[_expressions.size()];
-    _dictionaries = new Dictionary[_expressions.size()];
-    DataSchema.ColumnDataType[] columnDataTypes = new DataSchema.ColumnDataType[_expressions.size()];
-    String[] columnNames = new String[_expressions.size()];
-    for (int i = 0; i < _expressions.size(); i++) {
+    int numExpressions = _expressions.size();
+    _blockValSets = new BlockValSet[numExpressions];
+    String[] columnNames = new String[numExpressions];
+    ColumnDataType[] columnDataTypes = new ColumnDataType[numExpressions];
+    for (int i = 0; i < numExpressions; i++) {
       TransformExpressionTree expression = _expressions.get(i);
-      _expressionResultMetadata[i] = _transformOperator.getResultMetadata(expression);
-      columnDataTypes[i] = DataSchema.ColumnDataType
-          .fromDataType(_expressionResultMetadata[i].getDataType(), _expressionResultMetadata[i].isSingleValue());
       columnNames[i] = expression.toString();
-      if (_expressionResultMetadata[i].hasDictionary()) {
-        _dictionaries[i] = _transformOperator.getDictionary(expression);
-      }
+      TransformResultMetadata resultMetadata = _transformOperator.getResultMetadata(expression);
+      columnDataTypes[i] = ColumnDataType.fromDataType(resultMetadata.getDataType(), resultMetadata.isSingleValue());
     }
     _dataSchema = new DataSchema(columnNames, columnDataTypes);
-    if (_orderByExpressions.isEmpty()) {
-      _rowEvents = new ArrayList<>();
-    } else {
-      Comparator<Serializable[]> comparator = getStrictComparator();
-      _priorityQueue = new PriorityQueue<>(_maxRows, comparator);
-    }
   }
 
-  private Comparator<Serializable[]> getStrictComparator() {
-    return new Comparator<Serializable[]>() {
-      @Override
-      public int compare(Serializable[] o1, Serializable[] o2) {
-        List<SelectionSort> sortSequence = _selection.getSelectionSortSequence();
-        int numSortColumns = sortSequence.size();
-        for (int i = 0; i < numSortColumns; i++) {
-          int ret = 0;
-          SelectionSort selectionSort = sortSequence.get(i);
-          int index = _orderByIndices.get(i);
-          // Only compare single-value columns.
-          if (!_expressionResultMetadata[index].isSingleValue()) {
-            continue;
-          }
-
-          Serializable v1 = o1[index];
-          Serializable v2 = o2[index];
-
-          DataType dataType = _expressionResultMetadata[index].getDataType();
-          switch (dataType) {
-            case INT:
-              if (!selectionSort.isIsAsc()) {
-                ret = ((Integer) v1).compareTo((Integer) v2);
-              } else {
-                ret = ((Integer) v2).compareTo((Integer) v1);
-              }
-              break;
-            case LONG:
-              if (!selectionSort.isIsAsc()) {
-                ret = ((Long) v1).compareTo((Long) v2);
-              } else {
-                ret = ((Long) v2).compareTo((Long) v1);
-              }
-              break;
-            case FLOAT:
-              if (!selectionSort.isIsAsc()) {
-                ret = ((Float) v1).compareTo((Float) v2);
-              } else {
-                ret = ((Float) v2).compareTo((Float) v1);
-              }
-              break;
-            case DOUBLE:
-              if (!selectionSort.isIsAsc()) {
-                ret = ((Double) v1).compareTo((Double) v2);
-              } else {
-                ret = ((Double) v2).compareTo((Double) v1);
-              }
-              break;
-            case BOOLEAN:
-            case STRING:
-              if (!selectionSort.isIsAsc()) {
-                ret = ((String) v1).compareTo((String) v2);
-              } else {
-                ret = ((String) v2).compareTo((String) v1);
-              }
-              break;
-            case BYTES:
-              if (!selectionSort.isIsAsc()) {
-                ret = ByteArray.compare((byte[]) v1, (byte[]) v2);
-              } else {
-                ret = ByteArray.compare((byte[]) v2, (byte[]) v1);
-              }
-            default:
-              break;
-          }
+  // TODO: Optimize the comparator by comparing dictionary ids
+  private Comparator<Serializable[]> getComparator() {
+    return (o1, o2) -> {
+      int numSortExpressions = _sortSequence.size();
+      for (int i = 0; i < numSortExpressions; i++) {
+        // Only compare single-value columns
+        if (!_blockValSets[i].isSingleValue()) {
+          continue;
+        }
 
-          if (ret != 0) {
-            return ret;
-          }
+        Serializable v1 = o1[i];
+        Serializable v2 = o2[i];
+
+        int ret = 0;
+        SelectionSort selectionSort = _sortSequence.get(i);
+        switch (_blockValSets[i].getValueType()) {
+          case INT:
+            if (!selectionSort.isIsAsc()) {
+              ret = ((Integer) v1).compareTo((Integer) v2);
+            } else {
+              ret = ((Integer) v2).compareTo((Integer) v1);
+            }
+            break;
+          case LONG:
+            if (!selectionSort.isIsAsc()) {
+              ret = ((Long) v1).compareTo((Long) v2);
+            } else {
+              ret = ((Long) v2).compareTo((Long) v1);
+            }
+            break;
+          case FLOAT:
+            if (!selectionSort.isIsAsc()) {
+              ret = ((Float) v1).compareTo((Float) v2);
+            } else {
+              ret = ((Float) v2).compareTo((Float) v1);
+            }
+            break;
+          case DOUBLE:
+            if (!selectionSort.isIsAsc()) {
+              ret = ((Double) v1).compareTo((Double) v2);
+            } else {
+              ret = ((Double) v2).compareTo((Double) v1);
+            }
+            break;
+          case STRING:
+            if (!selectionSort.isIsAsc()) {
+              ret = ((String) v1).compareTo((String) v2);
+            } else {
+              ret = ((String) v2).compareTo((String) v1);
+            }
+            break;
+          case BYTES:
+            if (!selectionSort.isIsAsc()) {
+              ret = ByteArray.compare((byte[]) v1, (byte[]) v2);
+            } else {
+              ret = ByteArray.compare((byte[]) v2, (byte[]) v1);
+            }
+          default:
+            break;
+        }
+        if (ret != 0) {
+          return ret;
         }
-        return 0;
       }
+
+      return 0;
     };
   }
 
   @Override
   protected IntermediateResultsBlock getNextBlock() {
     int numDocsScanned = 0;
+    int numExpressions = _expressions.size();
+    boolean selectionOnly = _sortSequence.isEmpty();
 
     TransformBlock transformBlock;
-    boolean selectionOnly = _orderByExpressions.isEmpty();
     while ((transformBlock = _transformOperator.nextBlock()) != null) {
-      for (int i = 0; i < _expressions.size(); i++) {
+      for (int i = 0; i < numExpressions; i++) {
         TransformExpressionTree expression = _expressions.get(i);
         _blockValSets[i] = transformBlock.getBlockValueSet(expression);
       }
-      TransformBlockDataFetcher dataFetcher;
-      dataFetcher = new TransformBlockDataFetcher(_blockValSets, _dictionaries, _expressionResultMetadata);
+      TransformBlockDataFetcher dataFetcher = new TransformBlockDataFetcher(_blockValSets);
+      int numDocs = transformBlock.getNumDocs();
+      int docId = 0;
       if (selectionOnly) {
-        int docId = 0;
-        while (_rowEvents.size() < _limit && docId < transformBlock.getNumDocs()) {
+        // Selection only
+        while (_rows.size() < _limit && docId < numDocs) {
           Serializable[] row = dataFetcher.getRow(docId);
-          _rowEvents.add(row);
-          docId = docId + 1;
+          _rows.add(row);
+          docId++;
         }
-        if (_rowEvents.size() >= _limit) {
-          numDocsScanned += docId;
+        numDocsScanned += docId;
+        if (_rows.size() == _limit) {
           break;
         }
       } else {
-        //Selection + orderby
-        int docId = 0;
-        while (docId < transformBlock.getNumDocs()) {
+        // Selection order-by
+        while (docId < numDocs) {
           Serializable[] row = dataFetcher.getRow(docId);
           if (_priorityQueue.size() < _maxRows) {
             _priorityQueue.add(row);
@@ -247,7 +200,7 @@ public class SelectionOperator extends BaseOperator<IntermediateResultsBlock> {
             _priorityQueue.poll();
             _priorityQueue.offer(row);
           }
-          docId = docId + 1;
+          docId++;
         }
         numDocsScanned += docId;
       }
@@ -260,9 +213,12 @@ public class SelectionOperator extends BaseOperator<IntermediateResultsBlock> {
     _executionStatistics =
         new ExecutionStatistics(numDocsScanned, numEntriesScannedInFilter, numEntriesScannedPostFilter,
             numTotalRawDocs);
+
     if (selectionOnly) {
-      return new IntermediateResultsBlock(_dataSchema, _rowEvents);
+      // Selection only
+      return new IntermediateResultsBlock(_dataSchema, _rows);
     } else {
+      // Selection order-by
       return new IntermediateResultsBlock(_dataSchema, _priorityQueue);
     }
   }
@@ -277,6 +233,3 @@ public class SelectionOperator extends BaseOperator<IntermediateResultsBlock> {
     return _executionStatistics;
   }
 }
-
-
-
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
index b5c8d99..adf0684 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
@@ -18,320 +18,193 @@
  */
 package org.apache.pinot.core.operator.transform;
 
+import com.google.common.base.Preconditions;
 import java.io.Serializable;
-import org.apache.pinot.common.data.FieldSpec;
-import org.apache.pinot.common.data.FieldSpec.DataType;
-import org.apache.pinot.common.request.transform.TransformExpressionTree;
-import org.apache.pinot.common.utils.BytesUtils;
-import org.apache.pinot.common.utils.primitive.ByteArray;
 import org.apache.pinot.core.common.BlockValSet;
-import org.apache.pinot.core.segment.index.readers.Dictionary;
-
-public class TransformBlockDataFetcher {
 
 
+public class TransformBlockDataFetcher {
   private final Fetcher[] _fetchers;
 
-  public TransformBlockDataFetcher(BlockValSet[] blockValSets, Dictionary[] dictionaries,
-      TransformResultMetadata[] expressionResultMetadata) {
-    _fetchers = new Fetcher[blockValSets.length];
-    for (int i = 0; i < blockValSets.length; i++) {
-      _fetchers[i] = createFetcher(blockValSets[i], dictionaries[i],
-          expressionResultMetadata[i]);
+  public TransformBlockDataFetcher(BlockValSet[] blockValSets) {
+    int numColumns = blockValSets.length;
+    _fetchers = new Fetcher[numColumns];
+    for (int i = 0; i < numColumns; i++) {
+      _fetchers[i] = createFetcher(blockValSets[i]);
     }
   }
 
   public Serializable[] getRow(int docId) {
-    Serializable[] row = new Serializable[_fetchers.length];
-    for (int i = 0; i < _fetchers.length; i++) {
-      row[i] = _fetchers[i].getValue(docId);
-    }
-    return row;
+    return getRow(docId, new Serializable[_fetchers.length]);
   }
 
-  Fetcher createFetcher(BlockValSet blockValSet,
-      Dictionary dictionary,
-      TransformResultMetadata expressionResultMetadata) {
-    if (expressionResultMetadata.hasDictionary()) {
-      if (expressionResultMetadata.isSingleValue()) {
-        return new DictionaryBasedSVValueFetcher(dictionary,
-            blockValSet.getDictionaryIdsSV(), expressionResultMetadata.getDataType());
-      } else {
-        switch (expressionResultMetadata.getDataType()) {
-          case INT:
-            return new DictionaryBasedMVIntValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-          case LONG:
-            return new DictionaryBasedMVLongValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-          case FLOAT:
-            return new DictionaryBasedMVFloatValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-          case DOUBLE:
-            return new DictionaryBasedMVDoubleValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-          case BOOLEAN:
-          case STRING:
-            return new DictionaryBasedMVStringValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-          case BYTES:
-            return new DictionaryBasedMVBytesValueFetcher(dictionary,
-                blockValSet.getDictionaryIdsMV());
-        }
-      }
-    } else {
-      switch (expressionResultMetadata.getDataType()) {
-        case INT:
-          return new SVIntValueFetcher(blockValSet.getIntValuesSV());
-        case LONG:
-          return new SVLongValueFetcher(blockValSet.getLongValuesSV());
-        case FLOAT:
-          return new SVFloatValueFetcher(blockValSet.getFloatValuesSV());
-        case DOUBLE:
-          return new SVDoubleValueFetcher(blockValSet.getDoubleValuesSV());
-        case BOOLEAN:
-        case STRING:
-          return new SVStringValueFetcher(blockValSet.getStringValuesSV());
-        case BYTES:
-          return new SVBytesValueFetcher(blockValSet.getBytesValuesSV());
-      }
+  public Serializable[] getRow(int docId, Serializable[] reuse) {
+    int numColumns = _fetchers.length;
+    for (int i = 0; i < numColumns; i++) {
+      reuse[i] = _fetchers[i].getValue(docId);
+    }
+    return reuse;
+  }
+
+  private Fetcher createFetcher(BlockValSet blockValSet) {
+    boolean singleValue = blockValSet.isSingleValue();
+    switch (blockValSet.getValueType()) {
+      case INT:
+        return singleValue ? new IntSingleValueFetcher(blockValSet.getIntValuesSV())
+            : new IntMultiValueFetcher(blockValSet.getIntValuesMV());
+      case LONG:
+        return singleValue ? new LongSingleValueFetcher(blockValSet.getLongValuesSV())
+            : new LongMultiValueFetcher(blockValSet.getLongValuesMV());
+      case FLOAT:
+        return singleValue ? new FloatSingleValueFetcher(blockValSet.getFloatValuesSV())
+            : new FloatMultiValueFetcher(blockValSet.getFloatValuesMV());
+      case DOUBLE:
+        return singleValue ? new DoubleSingleValueFetcher(blockValSet.getDoubleValuesSV())
+            : new DoubleMultiValueFetcher(blockValSet.getDoubleValuesMV());
+      case STRING:
+        return singleValue ? new StringSingleValueFetcher(blockValSet.getStringValuesSV())
+            : new StringMultiValueFetcher(blockValSet.getStringValuesMV());
+      case BYTES:
+        Preconditions.checkState(singleValue);
+        return new BytesValueFetcher(blockValSet.getBytesValuesSV());
+      default:
+        throw new IllegalStateException();
     }
-    throw new UnsupportedOperationException();
-  }
-}
-
-
-interface Fetcher {
-
-  Serializable getValue(int docId);
-}
-
-class SVIntValueFetcher implements Fetcher {
-
-  private int[] _values;
-
-  SVIntValueFetcher(int[] values) {
-    _values = values;
-  }
-
-  public Serializable getValue(int docId) {
-    return _values[docId];
-  }
-}
-
-class SVLongValueFetcher implements Fetcher {
-
-  private long[] _values;
-
-  SVLongValueFetcher(long[] values) {
-    _values = values;
   }
 
-  public Serializable getValue(int docId) {
-    return _values[docId];
+  private interface Fetcher {
+    Serializable getValue(int docId);
   }
-}
-
-class SVFloatValueFetcher implements Fetcher {
 
-  private float[] _values;
+  private class IntSingleValueFetcher implements Fetcher {
+    private final int[] _values;
 
-  SVFloatValueFetcher(float[] values) {
-    _values = values;
-  }
+    IntSingleValueFetcher(int[] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    return _values[docId];
+    public Serializable getValue(int docId) {
+      return _values[docId];
+    }
   }
-}
-
-class SVDoubleValueFetcher implements Fetcher {
 
-  private double[] _values;
+  private class IntMultiValueFetcher implements Fetcher {
+    private final int[][] _values;
 
-  SVDoubleValueFetcher(double[] values) {
-    _values = values;
-  }
+    IntMultiValueFetcher(int[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    return _values[docId];
+    public Serializable getValue(int docId) {
+      return _values[docId];
+    }
   }
-}
-
-class SVStringValueFetcher implements Fetcher {
 
-  private String[] _values;
+  private class LongSingleValueFetcher implements Fetcher {
+    private final long[] _values;
 
-  SVStringValueFetcher(String[] values) {
-    _values = values;
-  }
+    LongSingleValueFetcher(long[] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    return _values[docId];
+    public Serializable getValue(int docId) {
+      return _values[docId];
+    }
   }
-}
 
-class SVBytesValueFetcher implements Fetcher {
+  private class LongMultiValueFetcher implements Fetcher {
+    private final long[][] _values;
 
-  private byte[][] _values;
-
-  SVBytesValueFetcher(byte[][] values) {
-    _values = values;
-  }
+    LongMultiValueFetcher(long[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    return _values[docId];
+    public Serializable getValue(int docId) {
+      return _values[docId];
+    }
   }
-}
-
-class DictionaryBasedSVValueFetcher implements Fetcher {
-
-  private Dictionary _dictionary;
-  private int[] _dictionaryIds;
-  private DataType _dataType;
 
-  DictionaryBasedSVValueFetcher(Dictionary dictionary, int[] dictionaryIds,
-      DataType dataType) {
+  private class FloatSingleValueFetcher implements Fetcher {
+    private final float[] _values;
 
-    _dictionary = dictionary;
-    _dictionaryIds = dictionaryIds;
-    _dataType = dataType;
-  }
+    FloatSingleValueFetcher(float[] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    return (Serializable) _dictionary.get(_dictionaryIds[docId]);
+    public Serializable getValue(int docId) {
+      return _values[docId];
+    }
   }
-}
-
 
-class DictionaryBasedMVIntValueFetcher implements Fetcher {
+  private class FloatMultiValueFetcher implements Fetcher {
+    private final float[][] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVIntValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    FloatMultiValueFetcher(float[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    int[] values = new int[dictIds.length];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getIntValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
-}
 
-class DictionaryBasedMVLongValueFetcher implements Fetcher {
+  private class DoubleSingleValueFetcher implements Fetcher {
+    private final double[] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVLongValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    DoubleSingleValueFetcher(double[] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    long[] values = new long[dictIds.length];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getLongValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
-}
 
-class DictionaryBasedMVFloatValueFetcher implements Fetcher {
+  private class DoubleMultiValueFetcher implements Fetcher {
+    private final double[][] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVFloatValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    DoubleMultiValueFetcher(double[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    float[] values = new float[dictIds.length];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getFloatValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
-}
 
-class DictionaryBasedMVDoubleValueFetcher implements Fetcher {
+  private class StringSingleValueFetcher implements Fetcher {
+    private final String[] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVDoubleValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    StringSingleValueFetcher(String[] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    double[] values = new double[dictIds.length];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getDoubleValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
-}
 
-class DictionaryBasedMVStringValueFetcher implements Fetcher {
+  private class StringMultiValueFetcher implements Fetcher {
+    private final String[][] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVStringValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    StringMultiValueFetcher(String[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    String[] values = new String[dictIds.length];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getStringValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
-}
 
-class DictionaryBasedMVBytesValueFetcher implements Fetcher {
+  private class BytesValueFetcher implements Fetcher {
+    private final byte[][] _values;
 
-  private Dictionary _dictionary;
-  private int[][] _dictionaryIdsArray;
-
-  DictionaryBasedMVBytesValueFetcher(Dictionary dictionary, int[][] dictionaryIdsArray) {
-
-    _dictionary = dictionary;
-    _dictionaryIdsArray = dictionaryIdsArray;
-  }
+    BytesValueFetcher(byte[][] values) {
+      _values = values;
+    }
 
-  public Serializable getValue(int docId) {
-    int[] dictIds = _dictionaryIdsArray[docId];
-    byte[][] values = new byte[dictIds.length][];
-    for (int i = 0; i < dictIds.length; i++) {
-      int dictId = dictIds[i];
-      values[i] = _dictionary.getBytesValue(dictId);
+    public Serializable getValue(int docId) {
+      return _values[docId];
     }
-    return values;
   }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformOperator.java
index 2081684..b4203ed 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformOperator.java
@@ -21,8 +21,6 @@ package org.apache.pinot.core.operator.transform;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import javax.annotation.Nonnull;
 import org.apache.pinot.common.request.transform.TransformExpressionTree;
 import org.apache.pinot.core.common.DataSource;
 import org.apache.pinot.core.operator.BaseOperator;
@@ -39,7 +37,6 @@ import org.apache.pinot.core.segment.index.readers.Dictionary;
  * Class for evaluating transform expressions.
  */
 public class TransformOperator extends BaseOperator<TransformBlock> {
-
   private static final String OPERATOR_NAME = "TransformOperator";
 
   private final ProjectionOperator _projectionOperator;
@@ -48,21 +45,19 @@ public class TransformOperator extends BaseOperator<TransformBlock> {
   private final List<TransformExpressionTree> _expressions;
 
   /**
-   * Constructor for the class
+   * Constructor for the class.
    *
    * @param projectionOperator Projection operator
    * @param expressions Set of expressions to evaluate
    */
-  public TransformOperator(@Nonnull ProjectionOperator projectionOperator,
-      @Nonnull List<TransformExpressionTree> expressions) {
+  public TransformOperator(ProjectionOperator projectionOperator, List<TransformExpressionTree> expressions) {
     _projectionOperator = projectionOperator;
-    _expressions = expressions;
     _dataSourceMap = projectionOperator.getDataSourceMap();
     for (TransformExpressionTree expression : expressions) {
-      TransformFunction transformFunction = TransformFunctionFactory
-          .get(expression, _dataSourceMap);
+      TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
       _transformFunctionMap.put(expression, transformFunction);
     }
+    _expressions = expressions;
   }
 
   /**
@@ -80,7 +75,7 @@ public class TransformOperator extends BaseOperator<TransformBlock> {
    * @param expression Expression
    * @return Transform result metadata
    */
-  public TransformResultMetadata getResultMetadata(@Nonnull TransformExpressionTree expression) {
+  public TransformResultMetadata getResultMetadata(TransformExpressionTree expression) {
     return _transformFunctionMap.get(expression).getResultMetadata();
   }
 
@@ -91,7 +86,7 @@ public class TransformOperator extends BaseOperator<TransformBlock> {
    *
    * @return Dictionary
    */
-  public Dictionary getDictionary(@Nonnull TransformExpressionTree expression) {
+  public Dictionary getDictionary(TransformExpressionTree expression) {
     return _transformFunctionMap.get(expression).getDictionary();
   }
 
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/plan/SelectionPlanNode.java b/pinot-core/src/main/java/org/apache/pinot/core/plan/SelectionPlanNode.java
index ef6db3b..46b77e7 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/SelectionPlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/SelectionPlanNode.java
@@ -54,7 +54,8 @@ public class SelectionPlanNode implements PlanNode {
     if (_selection.getSize() <= 0) {
       return new EmptySelectionOperator(_indexSegment, _selection);
     }
-    return new SelectionOperator(_indexSegment, _selection, _transformPlanNode.run());
+    return new SelectionOperator(_indexSegment, _selection, _transformPlanNode.run(),
+        _transformPlanNode.getSortSequence());
   }
 
   @Override
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java b/pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java
index c1e19d7..272cb84 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java
@@ -18,16 +18,11 @@
  */
 package org.apache.pinot.core.plan;
 
-import static org.apache.pinot.core.query.selection.SelectionOperatorUtils.getSelectionColumns;
-
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
-import javax.annotation.Nonnull;
+import org.apache.pinot.common.function.AggregationFunctionType;
 import org.apache.pinot.common.request.AggregationInfo;
 import org.apache.pinot.common.request.BrokerRequest;
 import org.apache.pinot.common.request.Selection;
@@ -35,7 +30,6 @@ import org.apache.pinot.common.request.SelectionSort;
 import org.apache.pinot.common.request.transform.TransformExpressionTree;
 import org.apache.pinot.core.indexsegment.IndexSegment;
 import org.apache.pinot.core.operator.transform.TransformOperator;
-import org.apache.pinot.common.function.AggregationFunctionType;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunctionUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -50,78 +44,88 @@ public class TransformPlanNode implements PlanNode {
   private final String _segmentName;
   private final ProjectionPlanNode _projectionPlanNode;
   private final Set<String> _projectionColumns = new HashSet<>();
-  private final Set<TransformExpressionTree> _expressionTrees = new LinkedHashSet<>();
+  private final List<TransformExpressionTree> _expressions = new ArrayList<>();
+  private final List<SelectionSort> _sortSequence = new ArrayList<>();
   private int _maxDocPerNextCall = DocIdSetPlanNode.MAX_DOC_PER_CALL;
 
-  /**
-   * Constructor for the class
-   *
-   * @param indexSegment Segment to process
-   * @param brokerRequest BrokerRequest to process
-   */
-  public TransformPlanNode(@Nonnull IndexSegment indexSegment, @Nonnull BrokerRequest brokerRequest) {
+  public TransformPlanNode(IndexSegment indexSegment, BrokerRequest brokerRequest) {
     _segmentName = indexSegment.getSegmentName();
-    extractColumnsAndTransforms(brokerRequest, indexSegment);
-    _projectionPlanNode =
-        new ProjectionPlanNode(indexSegment, _projectionColumns, new DocIdSetPlanNode(indexSegment, brokerRequest, _maxDocPerNextCall));
+    extractColumnsAndTransforms(indexSegment, brokerRequest);
+    _projectionPlanNode = new ProjectionPlanNode(indexSegment, _projectionColumns,
+        new DocIdSetPlanNode(indexSegment, brokerRequest, _maxDocPerNextCall));
   }
 
   /**
    * Helper method to extract projection columns and transform expressions from the given broker request.
-   *
-   * @param brokerRequest Broker request to process
-   * @param indexSegment
    */
-  private void extractColumnsAndTransforms(@Nonnull BrokerRequest brokerRequest,
-      IndexSegment indexSegment) {
+  private void extractColumnsAndTransforms(IndexSegment indexSegment, BrokerRequest brokerRequest) {
     if (brokerRequest.isSetAggregationsInfo()) {
+      // Aggregation queries
+
       for (AggregationInfo aggregationInfo : brokerRequest.getAggregationsInfo()) {
         if (!aggregationInfo.getAggregationType().equalsIgnoreCase(AggregationFunctionType.COUNT.getName())) {
           String expression = AggregationFunctionUtils.getColumn(aggregationInfo);
-          TransformExpressionTree transformExpressionTree = TransformExpressionTree.compileToExpressionTree(expression);
-          transformExpressionTree.getColumns(_projectionColumns);
-          _expressionTrees.add(transformExpressionTree);
+          TransformExpressionTree expressionTree = TransformExpressionTree.compileToExpressionTree(expression);
+          if (!_expressions.contains(expressionTree)) {
+            expressionTree.getColumns(_projectionColumns);
+            _expressions.add(expressionTree);
+          }
         }
       }
 
       // Process all group-by expressions
       if (brokerRequest.isSetGroupBy()) {
         for (String expression : brokerRequest.getGroupBy().getExpressions()) {
-          TransformExpressionTree transformExpressionTree = TransformExpressionTree.compileToExpressionTree(expression);
-          transformExpressionTree.getColumns(_projectionColumns);
-          _expressionTrees.add(transformExpressionTree);
+          TransformExpressionTree expressionTree = TransformExpressionTree.compileToExpressionTree(expression);
+          if (!_expressions.contains(expressionTree)) {
+            expressionTree.getColumns(_projectionColumns);
+            _expressions.add(expressionTree);
+          }
         }
       }
     } else {
+      // Selection queries
+
+      // Deduplicate sort expressions, and put them in front of other expressions
       Selection selection = brokerRequest.getSelections();
-      // No ordering required, select minimum number of documents
-      if (!selection.isSetSelectionSortSequence()) {
-        _maxDocPerNextCall = Math.min(selection.getOffset() + selection.getSize(), _maxDocPerNextCall);
-      }
-      List<String> expressions = selection.getSelectionColumns();
-      if (expressions.size() == 1 && expressions.get(0).equals("*")) {
-        expressions = new LinkedList<>(indexSegment.getPhysicalColumnNames());
-        Collections.sort(expressions);
-      }
-      if (selection.getSelectionSortSequence() != null) {
-        for (SelectionSort selectionSort : selection.getSelectionSortSequence()) {
+      List<SelectionSort> sortSequence = selection.getSelectionSortSequence();
+      if (sortSequence == null || sortSequence.isEmpty()) {
+        // No ordering required, select minimum number of documents
+        _maxDocPerNextCall = Math.min(selection.getSize(), _maxDocPerNextCall);
+      } else {
+        for (SelectionSort selectionSort : sortSequence) {
           String expression = selectionSort.getColumn();
-          if(!expressions.contains(expression)) {
-            expressions.add(expression);
+          TransformExpressionTree expressionTree = TransformExpressionTree.compileToExpressionTree(expression);
+          if (!_expressions.contains(expressionTree)) {
+            expressionTree.getColumns(_projectionColumns);
+            _expressions.add(expressionTree);
+            _sortSequence.add(selectionSort);
           }
         }
       }
-      for (String expression : expressions) {
-        TransformExpressionTree transformExpressionTree = TransformExpressionTree.compileToExpressionTree(expression);
-        transformExpressionTree.getColumns(_projectionColumns);
-        _expressionTrees.add(transformExpressionTree);
+
+      List<String> selectionColumns = selection.getSelectionColumns();
+      if (selectionColumns.size() == 1 && selectionColumns.get(0).equals("*")) {
+        selectionColumns = new ArrayList<>(indexSegment.getPhysicalColumnNames());
+        selectionColumns.sort(null);
+      }
+      for (String expression : selectionColumns) {
+        TransformExpressionTree expressionTree = TransformExpressionTree.compileToExpressionTree(expression);
+        if (!_expressions.contains(expressionTree)) {
+          expressionTree.getColumns(_projectionColumns);
+          _expressions.add(expressionTree);
+        }
       }
     }
   }
 
+  public List<SelectionSort> getSortSequence() {
+    return _sortSequence;
+  }
+
   @Override
   public TransformOperator run() {
-    return new TransformOperator(_projectionPlanNode.run(), new ArrayList<>(_expressionTrees));
+    return new TransformOperator(_projectionPlanNode.run(), new ArrayList<>(_expressions));
   }
 
   @Override


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